Cellular: Removed boilerplate code #10703

AnttiKauppila · 2019-05-29T13:20:55Z

Description

Some boiler plate code removed to reduce memory footprint. This is done by adding new functions to ATHandler so no API breaks are introduced. Only BG96 target is updated to take new functions into use. Other targets are using existing methods.

For BG96 this reduces flash size about 1,3kB

Pull request type

[ ] Fix
[X] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@AriParkkila @kivaisan @jarvte @mirelachirica

Release Notes

0Grit · 2019-05-29T13:53:25Z

Any chance of HE910 driver getting updated for this refactor? @maclobdell

AnttiKauppila · 2019-05-29T13:55:19Z

@loverdeg-ep All other targets will be updated accordingly but not in this PR.

AnttiKauppila · 2019-05-29T13:59:42Z

Internal JIRA reference: IOTCELL-2030

ciarmcom · 2019-05-29T15:01:58Z

@AnttiKauppila, thank you for your changes.
@AriParkkila @mirelachirica @jarvte @kivaisan @ARMmbed/mbed-os-wan @ARMmbed/mbed-os-maintainers please review.

features/cellular/framework/AT/AT_CellularStack.cpp

AnttiKauppila · 2019-06-02T15:57:55Z

Thanks @mirelachirica. Fixed now

jarvte · 2019-06-03T06:37:59Z

features/cellular/framework/AT/ATHandler.h

+     * @param resp_buf_size Response buffer size
+     * @param format Format string for variadic arguments to be added to AT command; No separator needed.
+     *        Use %d for integer, %s for string and %b for byte string (requires 2 arguments: string and length)
+     * @return @return last error that happened when parsing AT responses


extra @return, same with some other new doxygens.

jarvte · 2019-06-03T06:41:50Z

features/cellular/framework/AT/ATHandler.h

+     * @brief at_cmd_discard Send an AT command and read and discard a response. Locks and unlocks ATHandler for operation
+     * @param cmd AT command in form +<CMD> (will be used also in response reading, no extra chars allowed)
+     * @param cmd_chr Char to be added to specific AT command: '?', '=' or ''. Will be used as such so '=1' is valid as well.
+     * @param resp Integer to hold response


there is no resp argument

jarvte · 2019-06-03T07:26:11Z

features/cellular/framework/AT/AT_CellularNetwork.cpp

+        NWRegisteringMode mode;
+        get_network_registering_mode(mode);
+
+        if (mode != NWModeAutomatic) {


if mode == NWModeAutomatic then this function does not return anything

jarvte · 2019-06-03T07:33:34Z

features/cellular/framework/AT/AT_CellularNetwork.cpp

@@ -361,8 +325,7 @@ nsapi_error_t AT_CellularNetwork::scan_plmn(operList_t &operators, int &opsCount

    _at.lock();

-    _at.cmd_start("AT+COPS=?");
-    _at.cmd_stop();
+    _at.cmd_start_stop("+COPS", "?");


missing '=' ?

40Grit · 2019-06-03T11:17:50Z

This is a nit, but the title of this PR has been bothering me.

I'm thinking more along the lines of the following
'''
"Cellular: Refactor 'x' to eliminate redundant logic"
'''
But even that doesn't quite capture the essence of this PR.

@0xc0170

AnttiKauppila · 2019-06-04T07:48:44Z

@jarvte good catches, fixed now.

UNITTESTS/stubs/ATHandler_stub.cpp

features/cellular/framework/AT/AT_CellularContext.cpp

0xc0170 · 2019-06-13T11:47:04Z

CI restarted

mbed-ci · 2019-06-13T11:51:24Z

Test run: FAILED

Summary: 1 of 1 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_unittests

0xc0170 · 2019-06-13T13:30:33Z

CI restarted

mbed-ci · 2019-06-13T14:33:58Z

Test run: FAILED

Summary: 1 of 7 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-IAR

0xc0170 · 2019-06-14T08:53:05Z

There's fix proposed in #10833, will restart CI after

AnttiKauppila · 2019-06-14T09:00:46Z

IAR Fastmodel was broken and is fixed: #10833.
@adbridge @0xc0170 Please retrigger build

0xc0170 · 2019-06-14T09:04:24Z

as soon as the fix lands , will do

AnttiKauppila · 2019-06-14T09:06:38Z

Sorry it is not yet merged :(

adbridge · 2019-06-15T09:43:01Z

CI started

mbed-ci · 2019-06-15T14:48:00Z

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 4
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

adbridge · 2019-06-17T10:31:23Z

Seem to be many random CI failures over the weekend, restarting CI

mbed-ci · 2019-06-17T15:57:29Z

Test run: FAILED

Summary: 2 of 11 test jobs failed
Build number : 5
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_exporter
jenkins-ci/mbed-os-ci_greentea-test

adbridge · 2019-06-18T15:31:17Z

CI started

mbed-ci · 2019-06-18T18:31:15Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 6
Build artifacts

AnttiKauppila · 2019-06-19T09:58:05Z

@adbridge @0xc0170 Can you merge this?

Cellular: Removed boiler plate code

77e8374

AnttiKauppila force-pushed the optimisation branch 2 times, most recently from 92a285a to 9ddf3ad Compare May 29, 2019 13:45

AnttiKauppila force-pushed the optimisation branch from 9ddf3ad to e2b7cd9 Compare May 29, 2019 13:53

AnttiKauppila force-pushed the optimisation branch from e2b7cd9 to d3d8997 Compare May 29, 2019 14:11

ciarmcom requested review from AriParkkila, jarvte, kivaisan, mirelachirica and a team May 29, 2019 15:01

ciarmcom added the needs: review label May 29, 2019

astyle fixes

83d62fb

AnttiKauppila force-pushed the optimisation branch from d3d8997 to 83d62fb Compare May 29, 2019 17:13

mirelachirica suggested changes May 30, 2019

View reviewed changes

features/cellular/framework/AT/AT_CellularStack.cpp Outdated Show resolved Hide resolved

0xc0170 added needs: work release-version: 5.14.0-rc1 and removed needs: review labels May 31, 2019

Added missing _cid handling

7f199ce

jarvte suggested changes Jun 3, 2019

View reviewed changes

Small fixes and doxygen update

9d8969d

AnttiKauppila changed the title ~~Cellular: Removed boiler plate code~~ Cellular: Removed boilerplate code Jun 4, 2019

0xc0170 removed the needs: work label Jun 4, 2019

adbridge requested changes Jun 12, 2019

View reviewed changes

UNITTESTS/stubs/ATHandler_stub.cpp Outdated Show resolved Hide resolved

UNITTESTS/stubs/ATHandler_stub.cpp Outdated Show resolved Hide resolved

features/cellular/framework/AT/AT_CellularContext.cpp Outdated Show resolved Hide resolved

AnttiKauppila force-pushed the optimisation branch from 1d65ade to 07ea91e Compare June 12, 2019 11:58

0xc0170 requested a review from adbridge June 13, 2019 11:40

0xc0170 added needs: review and removed needs: work labels Jun 13, 2019

Valgrind & unittest fixes

1ed338e

AnttiKauppila force-pushed the optimisation branch from 07ea91e to 1ed338e Compare June 13, 2019 12:16

adbridge approved these changes Jun 13, 2019

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels Jun 13, 2019

adbridge added ready for merge and removed needs: CI labels Jun 19, 2019

adbridge merged commit 6000724 into ARMmbed:master Jun 19, 2019

adbridge removed the ready for merge label Jun 19, 2019

AnttiKauppila deleted the optimisation branch June 19, 2019 12:13

Cellular: Removed boilerplate code #10703

Cellular: Removed boilerplate code #10703

Uh oh!

Conversation

AnttiKauppila commented May 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Reviewers

Release Notes

Uh oh!

0Grit commented May 29, 2019

Uh oh!

AnttiKauppila commented May 29, 2019

Uh oh!

AnttiKauppila commented May 29, 2019

Uh oh!

ciarmcom commented May 29, 2019

Uh oh!

Uh oh!

AnttiKauppila commented Jun 2, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

40Grit commented Jun 3, 2019

Uh oh!

AnttiKauppila commented Jun 4, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

0xc0170 commented Jun 13, 2019

Uh oh!

mbed-ci commented Jun 13, 2019

Uh oh!

0xc0170 commented Jun 13, 2019

Uh oh!

mbed-ci commented Jun 13, 2019

Uh oh!

0xc0170 commented Jun 14, 2019

Uh oh!

AnttiKauppila commented Jun 14, 2019

Uh oh!

0xc0170 commented Jun 14, 2019

Uh oh!

AnttiKauppila commented Jun 14, 2019

Uh oh!

adbridge commented Jun 15, 2019

Uh oh!

mbed-ci commented Jun 15, 2019

Uh oh!

adbridge commented Jun 17, 2019

Uh oh!

mbed-ci commented Jun 17, 2019

Uh oh!

adbridge commented Jun 18, 2019

Uh oh!

mbed-ci commented Jun 18, 2019

Uh oh!

AnttiKauppila commented Jun 19, 2019

Uh oh!

Uh oh!

AnttiKauppila commented May 29, 2019 •

edited

Loading