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

Merge feature cellular refactor #9568

Merged
merged 44 commits into from Feb 7, 2019

Conversation

Projects
None yet
@AriParkkila
Copy link
Contributor

commented Jan 31, 2019

Description

Merge the feature-cellular-refactor branch to mbed-os/master.

This is a breaking change and needed due to without this cellular stack cannot for example support upcoming multihoming requirement, see a comment below.

Pull request type

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

Reviewers

Release Notes

This PR has impact on cellular application development and porting of new cellular devices.

Application developers need to select the suitable NetworkInterface type. When using NetworkInterface NSAPI configuration defines are applied by default, see NSAPI configuration options for cellular in more details. CellularBase users have a new function set_default_parameters to apply NSAPI configuration options. CellularContext is to be used instead of CellularBase for new extended cellular functionality, such as NonIP sockets and EPS CP optimization...

Migration guidelines for application developers:

  • Change CELLULAR_DEVICE macro to get_default_instance
  • Change OnboardCellularInterface or EasyCellularConnection to CellularContext
  • Call CellularDevice::set_data_carrier_detect to set HUP signal detection

Cellular API changes need to be taken into account when porting new cellular devices. This PR already contains the changes needed for the existing cellular drivers and the onboard modems. Arm has an internal ticket to update the cellular porting documentation for Mbed OS 5.12.

Migration guidelines for porting developers:

  • Follow GENERIC_AT3GPP implementation when porting new AT based cellular devices
  • Change power_on/off to onboard_modem_api with soft_power_on/off and hard_power_on/off
  • Move implementation from CellularSIM and CellularPower to CellularInformation and CellularDevice
  • Implement CellularDevice::init and CellularDevice::shutdown
  • Use CellularNetwork::get_signal_quality instead of CellularNetwork::get_extended_signal_quality

Teemu Kultala and others added some commits Nov 9, 2018

cellular: eps ciot optimization network support check
-added an API for checking network eps ciot optimization support
-renamed the API for getting the UE parameters
-the API for setting the UE parameters includes now a callback, which
will be called once network support for eps ciot optimization is known
Removed CellularSIM interface.
Moved methods to classes CellularDevice and CellularInformation.
SIM interface was removed to simplify cellular usage and
methods better suite new classes.
Updated greentea and unit tests.
Cellular: fix possible crash in state machine
_sim_pin was changed to pointer from array and length was checked with
strlen. If _sim_pin was null it caused crash. Fix by checking _sim_pin against NULL.
Power class could have been called without checking if power is NULL. Fix by checking
that power class is not null.
Fix state machine to return correct states when queried.
Cellular: Change AT_CellularNetwork to use CellularProperties.
This change enables removing function has_registration from
class AT_CellularNetwork and all targets inheriting
AT_CellularNetwork.
Cellular: Remove target files inheriting from AT_CellularNetwork
After AT_CellularNetwork::has_registration was replaced with
CellularProperties and better
AT_CellularNetwork::set_access_technology_impl default
implementation we can delete most of the target specific classes
that inherit AT_CellularNetwork.
Cellular: change stack_type_supported to get_property
Change usage of AT_CellularContext::stack_type_supported to
AT_CellularBase::get_property. This way we can rid of
targets overriding stack_type_supported and delete
unnecessary classes and simplify new targets.
Cellular: Removed unnecessary checks after new
After this change we were able to change methods
ATHandler::set_urc_handler and CellularDevice::set_ready_cb to void
and simplify error handling.
Cellular: Added generic cellular modem
Generic cellular module (GENERIC_AT3GPP) can by used as a default
module when porting new cellular module. It's a good starting point
and eases porting of new modules. GENERIC_AT3GPP uses only standard
3GPP AT commands when communicating with the modem.
@AnttiKauppila

This comment has been minimized.

Copy link
Contributor

commented Feb 1, 2019

Hi @bulislaw, This is mandatory change needed for cellular stack, without this we cannot for example support upcoming multihoming requirement. This change has been widely discussed internally including Product management, PE team and Cellular team and we have created a list of changes we are about to do. @AriParkkila please make sure that all stakeholders will get the list of changes!
What I agree though is that the description of the PR is almost non-existent and causes this kind of uncertainty amongst people not aware of the background of this change.

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Feb 2, 2019

CI started.
Testing since CI load during weekend should be low, and I'd like to see what else might technically be needed for the PR

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Feb 2, 2019

@ARMmbed/mbed-os-maintainers another breaking change here without explanation

PR opened on 5:30am Jan 31st,
lack of a breaking change statement found Feb 1st ~6am. (#9568 (review))

@bulislaw Would you be interested in being a maintainer, or was the rapid response just a coincidense? 😄

@mbed-ci

This comment has been minimized.

Copy link

commented Feb 2, 2019

Test run: SUCCESS

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

@0xc0170

This comment has been minimized.

Copy link
Member

commented Feb 4, 2019

@AriParkkila Please can you write release notes section following this ARMmbed/mbed-os-5-docs#933 (will be integrated soon) - edit the first comment (introduction of PR, adding new release notes section) and let us know once updated

@AriParkkila

This comment has been minimized.

Copy link
Contributor Author

commented Feb 5, 2019

@0xc0170 please see Release Notes

@jarvte

This comment has been minimized.

Copy link
Contributor

commented Feb 5, 2019

@cmonr @0xc0170 @bulislaw can we proceed with this one? We need this to 5.12 and we need to still fix/add features and have time to test before 5.12.

@bulislaw bulislaw requested a review from AnttiKauppila Feb 5, 2019

@bulislaw

This comment has been minimized.

Copy link
Member

commented Feb 5, 2019

We need approval from @AnttiKauppila

@AriParkkila

This comment has been minimized.

Copy link
Contributor Author

commented Feb 6, 2019

@bulislaw do you think we can continue with merging this PR now?

@NirSonnenschein

This comment has been minimized.

Copy link
Contributor

commented Feb 6, 2019

starting CI

@theotherjimmy

This comment has been minimized.

Copy link
Contributor

commented Feb 6, 2019

Why was mbed-os-tools tagged for review? There are no tools changes to speak of.

@mbed-ci

This comment has been minimized.

Copy link

commented Feb 6, 2019

Test run: SUCCESS

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

@jarvte

This comment has been minimized.

Copy link
Contributor

commented Feb 7, 2019

Merge?

@0xc0170 0xc0170 removed the request for review from ARMmbed/mbed-os-tools Feb 7, 2019

@0xc0170

0xc0170 approved these changes Feb 7, 2019

@NirSonnenschein NirSonnenschein merged commit 8c2ad14 into master Feb 7, 2019

28 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
jenkins-ci/build-ARM Success
Details
jenkins-ci/build-ARMC6 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(+0 bytes) RAM(+0 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 Local astyle testing has passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/doxy-spellcheck Local doxy-spellcheck testing has passed
Details
travis-ci/events Passed, runtime is 10505 cycles
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/include_check Local include_check testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8408B
Details
travis-ci/psa-autogen Local psa-autogen testing has passed
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
travis-ci/tools-py3.5 Local tools-py3.5 testing has passed
Details
travis-ci/tools-py3.6 Local tools-py3.6 testing has passed
Details
travis-ci/tools-py3.7 Local tools-py3.7 testing has passed
Details
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.