Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cellular: Removed boilerplate code #10703

Merged
merged 7 commits into from Jun 19, 2019

Conversation

@AnttiKauppila
Copy link
Contributor

commented May 29, 2019

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

@AnttiKauppila AnttiKauppila force-pushed the AnttiKauppila:optimisation branch 2 times, most recently from 92a285a to 9ddf3ad May 29, 2019
@loverdeg-ep

This comment has been minimized.

Copy link
Contributor

commented May 29, 2019

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

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

This comment has been minimized.

Copy link
Contributor Author

commented May 29, 2019

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

@AnttiKauppila

This comment has been minimized.

Copy link
Contributor Author

commented May 29, 2019

Internal JIRA reference: IOTCELL-2030

@AnttiKauppila AnttiKauppila force-pushed the AnttiKauppila:optimisation branch from e2b7cd9 to d3d8997 May 29, 2019
@ciarmcom ciarmcom requested review from AriParkkila, jarvte, kivaisan, mirelachirica and ARMmbed/mbed-os-maintainers May 29, 2019
@ciarmcom

This comment has been minimized.

Copy link
Member

commented May 29, 2019

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

This comment has been minimized.

Copy link
Contributor Author

commented Jun 2, 2019

Thanks @mirelachirica. Fixed now

* @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

This comment has been minimized.

Copy link
@jarvte

jarvte Jun 3, 2019

Contributor

extra @return, same with some other new doxygens.

* @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

This comment has been minimized.

Copy link
@jarvte

jarvte Jun 3, 2019

Contributor

there is no resp argument

NWRegisteringMode mode;
get_network_registering_mode(mode);

if (mode != NWModeAutomatic) {

This comment has been minimized.

Copy link
@jarvte

jarvte Jun 3, 2019

Contributor

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

@@ -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", "?");

This comment has been minimized.

Copy link
@jarvte

jarvte Jun 3, 2019

Contributor

missing '=' ?

@40Grit

This comment has been minimized.

Copy link

commented Jun 3, 2019

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 AnttiKauppila changed the title Cellular: Removed boiler plate code Cellular: Removed boilerplate code Jun 4, 2019
@AnttiKauppila

This comment has been minimized.

Copy link
Contributor Author

commented Jun 4, 2019

@jarvte good catches, fixed now.

@0xc0170 0xc0170 removed the needs: work label Jun 4, 2019
@AnttiKauppila AnttiKauppila force-pushed the AnttiKauppila:optimisation branch from 1d65ade to 07ea91e Jun 12, 2019
@0xc0170 0xc0170 requested a review from adbridge Jun 13, 2019
@0xc0170 0xc0170 added needs: review and removed needs: work labels Jun 13, 2019
@0xc0170

This comment has been minimized.

Copy link
Member

commented Jun 13, 2019

CI restarted

@mbed-ci

This comment has been minimized.

Copy link

commented Jun 13, 2019

Test run: FAILED

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

Failed test jobs:

  • jenkins-ci/mbed-os-ci_unittests
@AnttiKauppila AnttiKauppila force-pushed the AnttiKauppila:optimisation branch from 07ea91e to 1ed338e Jun 13, 2019
@0xc0170 0xc0170 added needs: CI and removed needs: review labels Jun 13, 2019
@0xc0170

This comment has been minimized.

Copy link
Member

commented Jun 13, 2019

CI restarted

@mbed-ci

This comment has been minimized.

Copy link

commented Jun 13, 2019

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

This comment has been minimized.

Copy link
Member

commented Jun 14, 2019

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

@AnttiKauppila

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2019

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

@0xc0170

This comment has been minimized.

Copy link
Member

commented Jun 14, 2019

as soon as the fix lands , will do

@AnttiKauppila

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2019

Sorry it is not yet merged :(

@adbridge

This comment has been minimized.

Copy link
Contributor

commented Jun 15, 2019

CI started

@mbed-ci

This comment has been minimized.

Copy link

commented Jun 15, 2019

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

This comment has been minimized.

Copy link
Contributor

commented Jun 17, 2019

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

@mbed-ci

This comment has been minimized.

Copy link

commented Jun 17, 2019

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

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2019

CI started

@mbed-ci

This comment has been minimized.

Copy link

commented Jun 18, 2019

Test run: SUCCESS

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

@AnttiKauppila

This comment has been minimized.

Copy link
Contributor Author

commented Jun 19, 2019

@adbridge @0xc0170 Can you merge this?

@adbridge adbridge added ready for merge and removed needs: CI labels Jun 19, 2019
@adbridge adbridge merged commit 6000724 into ARMmbed:master Jun 19, 2019
26 checks passed
26 checks passed
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARM Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(+4488 bytes) RAM(+17104 bytes)
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARM Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Success!
Details
travis-ci/docs Success!
Details
travis-ci/doxy-spellcheck Success!
Details
travis-ci/events Success! Runtime is 8601 cycles.
Details
travis-ci/gitattributestest Success!
Details
travis-ci/include_check Success!
Details
travis-ci/licence_check Success!
Details
travis-ci/littlefs Success! Code size is 8448B.
Details
travis-ci/psa-autogen Success!
Details
travis-ci/tools-py2.7 Success!
Details
travis-ci/tools-py3.5 Success!
Details
travis-ci/tools-py3.6 Success!
Details
travis-ci/tools-py3.7 Success!
Details
@AnttiKauppila AnttiKauppila deleted the AnttiKauppila:optimisation branch Jun 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.