Refactored away onboard_modem_api because it is not needed at all #12092

AnttiKauppila · 2019-12-12T12:59:06Z

Summary of changes

Refactored away onboard_modem_api because it is not needed at all
All targets must implement soft_- and hard_power_on/off() functions which are practically same what onboard_modem_api offered.
These were seen as a duplicate features and therefore we removed this.
All targets involved have been updated to reflect the changes as this is "an internal feature break"

This will clean up the needed logic for Mbed OS internal targets which are defined in targets.json. Less configuration = easier to maintain for everybody!

Also moved non-IP socket related classes into netsocket root folder. This won't break anything due to our build system.

Impact of changes

All cellular modems having onboard modem are affected. Code has been updated to reflect the changes.

Migration actions required

onboard_modem_api.h removed

Documentation

Refactored away onboard_modem_api because it is not needed at all
All targets must implement soft_- and hard_power_on/off() functions which are practically same what onboard_modem_api offered.
These were seen as a duplicate features and therefore we removed this.
All targets involved have been updated to reflect the changes as this is "an internal feature break"

Pull request type

[X] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[X] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@ARMmbed/mbed-os-wan

targets/TARGET_STM/TARGET_STM32L4/TARGET_MTS_DRAGONFLY_L471QG/ONBOARD_SARA4_PPP.cpp.autosave

ciarmcom · 2019-12-12T14:00:34Z

@AnttiKauppila, thank you for your changes.
@maclobdell @ARMmbed/mbed-os-wan @ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-maintainers please review.

AriParkkila

Could still have qualified lookup for C functions..?

40Grit · 2019-12-13T10:52:31Z

@trowbridgec

0xc0170 · 2019-12-16T08:53:12Z

cc @ARMmbed/team-ublox @ARMmbed/team-multitech

kjbracey · 2019-12-17T11:55:43Z

CI started

AnttiKauppila · 2019-12-17T12:16:50Z

Unittests fixed

mbed-ci · 2019-12-17T12:35:30Z

Test run: FAILED

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

Failed test jobs:

jenkins-ci/mbed-os-ci_unittests
jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-IAR

trowbridgec · 2019-12-17T15:28:54Z

...NORDIC/TARGET_NRF5x/TARGET_NRF52/TARGET_MCU_NRF52840/TARGET_EP_AGORA/ONBOARD_TELIT_ME910.cpp

+
+    gpio_init_out_ex(&gpio, PIN_NAME_CELL_ON_OFF, 1);
+    gpio_write(&gpio, 0);
+    thread_sleep_for(time_ms);


Should the calls to thread_sleep_for() be changed to ThisThread::sleep_for() since this is now C++ and not C? I suppose the least amount of change the better to preserve functionality, but it's also good to follow convention. This would then apply to all affected modules as well.

This isn't a matter of convention - the C version is guaranteed always available, as it's in platform. The C++ one is in rtos-api, which might not be in a minimal build (via requires: "bare-metal" or .mbedignore rtos). So the C version is appropriate for HAL code.

Ah ok, thanks for the clarification.

trowbridgec · 2019-12-17T15:33:05Z

...T_NORDIC/TARGET_NRF5x/TARGET_NRF52/TARGET_MCU_NRF52840/TARGET_EP_AGORA/ONBOARD_TELIT_ME910.h

+
+    void onboard_modem_power_up();
+
+    void onboard_modem_power_down();


Should we consider making these functions public? Making them private restricts power optimization efforts at the application level which were previously available in the onboard_modem_api.c functions (e.g. turning off the cell module when not in use).

I would expect the power control from application end to be via higher-level calls to the cellular framework - those calls were always viewed as "driver core to hardware glue" type calls, and would be only part of the power saving. (You'd want to do serial port suspension too).

Whether higher-level application appropriate calls currently exist is another matter.

kjbracey · 2019-12-17T15:42:49Z

CI restarted

mbed-ci · 2019-12-17T16:22:28Z

Test run: FAILED

Summary: 3 of 4 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-IAR

adbridge · 2019-12-17T16:47:27Z

@AnttiKauppila it looks like a previous merge has caused this to need a rebase.

mbed-ci · 2019-12-19T11:33:26Z

Test run: FAILED

Summary: 3 of 4 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-IAR

All targets must implement soft_- and hard_power_on/off() functions which are practically same what onboard_modem_api offered. These were seen as a duplicate features and therefore we removed this. All targets involved have been updated to reflect the changes

AnttiKauppila · 2019-12-19T11:47:19Z

Build issue fixed

mbed-ci · 2019-12-19T12:34:05Z

Test run: FAILED

Summary: 3 of 4 test jobs failed
Build number : 4
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-IAR

AnttiKauppila · 2019-12-19T12:55:23Z

One more fix done

mbed-ci · 2019-12-19T14:11:13Z

Test run: SUCCESS

Summary: 12 of 12 test jobs passed
Build number : 5
Build artifacts

adbridge · 2019-12-19T15:24:27Z

Restarted ci after update

mbed-ci · 2019-12-19T16:16:33Z

Test run: FAILED

Summary: 1 of 12 test jobs failed
Build number : 6
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

kjbracey · 2019-12-20T08:06:40Z

DISCO L475 serial error. CI restarted

mbed-ci · 2019-12-20T08:59:09Z

Test run: SUCCESS

Summary: 12 of 12 test jobs passed
Build number : 7
Build artifacts

AnttiKauppila changed the title ~~Cellu refactor~~ Refactored away onboard_modem_api because it is not needed at all Dec 12, 2019

kivaisan approved these changes Dec 12, 2019

View reviewed changes

kimlep01 reviewed Dec 12, 2019

View reviewed changes

targets/TARGET_STM/TARGET_STM32L4/TARGET_MTS_DRAGONFLY_L471QG/ONBOARD_SARA4_PPP.cpp.autosave Outdated Show resolved Hide resolved

ciarmcom requested review from maclobdell and a team December 12, 2019 14:00

ciarmcom added the needs: review label Dec 12, 2019

AriParkkila approved these changes Dec 13, 2019

View reviewed changes

AnttiKauppila force-pushed the cellu_refactor branch from be39dbe to 4b3b135 Compare December 13, 2019 06:24

AnttiKauppila added needs: CI and removed needs: review labels Dec 13, 2019

0xc0170 added the release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 label Dec 16, 2019

0xc0170 approved these changes Dec 16, 2019

View reviewed changes

AnttiKauppila force-pushed the cellu_refactor branch from 4b3b135 to 8d63ebd Compare December 17, 2019 12:16

trowbridgec reviewed Dec 17, 2019

View reviewed changes

adbridge added needs: work and removed needs: CI labels Dec 17, 2019

AnttiKauppila force-pushed the cellu_refactor branch from 8d63ebd to d7de9c1 Compare December 18, 2019 06:30

kjbracey added needs: CI and removed needs: work labels Dec 19, 2019

Antti Kauppila added 3 commits December 19, 2019 13:44

relocated NonIP related files to netsocket root folder

e32d01f

Fixed Non-IP cellular socket unittest

7bfc16f

AnttiKauppila force-pushed the cellu_refactor branch from d7de9c1 to 4436abb Compare December 19, 2019 11:46

Added missing define for Quectel UG96

3a10982

AnttiKauppila force-pushed the cellu_refactor branch from 4436abb to 3a10982 Compare December 19, 2019 12:54

kjbracey added ready for merge and removed needs: CI labels Dec 23, 2019

bulislaw merged commit e29cb19 into ARMmbed:master Dec 27, 2019

0xc0170 removed the ready for merge label Jan 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactored away onboard_modem_api because it is not needed at all #12092

Refactored away onboard_modem_api because it is not needed at all #12092

AnttiKauppila commented Dec 12, 2019 •

edited

Loading

ciarmcom commented Dec 12, 2019

AriParkkila left a comment

40Grit commented Dec 13, 2019

0xc0170 commented Dec 16, 2019

kjbracey commented Dec 17, 2019

AnttiKauppila commented Dec 17, 2019

mbed-ci commented Dec 17, 2019

trowbridgec Dec 17, 2019

kjbracey Dec 17, 2019

trowbridgec Dec 17, 2019

trowbridgec Dec 17, 2019

kjbracey Dec 17, 2019

kjbracey commented Dec 17, 2019

mbed-ci commented Dec 17, 2019

adbridge commented Dec 17, 2019

mbed-ci commented Dec 19, 2019

AnttiKauppila commented Dec 19, 2019

mbed-ci commented Dec 19, 2019

AnttiKauppila commented Dec 19, 2019

mbed-ci commented Dec 19, 2019

adbridge commented Dec 19, 2019

mbed-ci commented Dec 19, 2019

kjbracey commented Dec 20, 2019

mbed-ci commented Dec 20, 2019


		void onboard_modem_power_up();

		void onboard_modem_power_down();

Refactored away onboard_modem_api because it is not needed at all #12092

Refactored away onboard_modem_api because it is not needed at all #12092

Conversation

AnttiKauppila commented Dec 12, 2019 • edited Loading

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

@ARMmbed/mbed-os-wan

ciarmcom commented Dec 12, 2019

AriParkkila left a comment

Choose a reason for hiding this comment

40Grit commented Dec 13, 2019

0xc0170 commented Dec 16, 2019

kjbracey commented Dec 17, 2019

AnttiKauppila commented Dec 17, 2019

mbed-ci commented Dec 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kjbracey commented Dec 17, 2019

mbed-ci commented Dec 17, 2019

adbridge commented Dec 17, 2019

mbed-ci commented Dec 19, 2019

AnttiKauppila commented Dec 19, 2019

mbed-ci commented Dec 19, 2019

AnttiKauppila commented Dec 19, 2019

mbed-ci commented Dec 19, 2019

adbridge commented Dec 19, 2019

mbed-ci commented Dec 19, 2019

kjbracey commented Dec 20, 2019

mbed-ci commented Dec 20, 2019

AnttiKauppila commented Dec 12, 2019 •

edited

Loading