Cellular refactor power and get_default_instance, also remove deprecated #9273

AriParkkila · 2019-01-07T11:27:22Z

Description

This PR has the following related changes:

Move power on/off to device and remove CellularPower
Remove netsocket/generic_modem_driver
Add get_target_default_instance in CellularDevice
Default modem drivers with FF_ARDUINO (see review comments)
Power API updated to match onboard_modem_api (see review comments)

CELLULAR_DEVICE macro and onboard_modem_api (see review comments) is still left to be removed after these changes can be approved.

Pull request type

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

Reviewers

@jarvte
@mirelachirica
@kjbracey-arm

…quality

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.

-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

Cellular: Remove sim interface

…n_feature_branch Cellular ciot eps on feature branch

Cell refactor power api

Cellular: changed support features to CellularProperty array.

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: Removed unnecessary checks after new

This change enables removing function has_registration from class AT_CellularNetwork and all targets inheriting 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.

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.

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.

_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: Add generic cellular modem

Cellular: added setting of data carrier support for UART.

ciarmcom · 2019-01-07T12:00:31Z

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

features/cellular/framework/targets/QUECTEL/UG96/mbed_lib.json

kjbracey

As discussed offline, I think it would be good to pin down exactly what the power calls do as we're making this API change - make sure we don't need to make another one.

Aligning with the 4-call model from the original 5.7 design, your power_on and power_off could be the "power rail" "init" and "deinit" from there.

The "button" power on could be hidden inside "init", but not clear that the button-off call can be neatly hidden inside "shutdown".

I would suggest exposing all 4 calls, and then the state machine can be in control of them all: "button on" calls interleaved with its "device init"s, and could in future choose when to try the "button off" if it ever did shutdown.

features/cellular/framework/AT/AT_CellularDevice.cpp

targets/TARGET_STM/TARGET_STM32L4/TARGET_MTS_DRAGONFLY_L471QG/ONBOARD_SARA4_PPP.cpp

...RGET_STM/TARGET_STM32L4/TARGET_STM32L486xG/TARGET_MTB_ADV_WISE_1570/ONBOARD_QUECTEL_BC95.cpp

features/cellular/framework/device/CellularStateMachine.cpp

features/cellular/framework/targets/GEMALTO/CINTERION/mbed_lib.json

features/cellular/framework/targets/QUECTEL/BG96/mbed_lib.json

features/cellular/framework/targets/GENERIC/GENERIC_AT3GPP/GENERIC_AT3GPP.h

targets/TARGET_STM/TARGET_STM32F4/TARGET_MTB_MTS_DRAGONFLY/ONBOARD_TELIT_HE910.cpp

AriParkkila · 2019-01-15T11:08:36Z

@kjbracey-arm please re-review

kjbracey

Looking good, thanks - I'd just like a bit more text in the power calls to help people get a mental image.

features/cellular/framework/API/CellularDevice.h

features/cellular/framework/targets/TELIT/HE910/TELIT_HE910.cpp

targets/TARGET_STM/TARGET_STM32F4/TARGET_STM32F439xI/TARGET_WIO_3G/ONBOARD_QUECTEL_BG96.cpp

features/cellular/framework/API/CellularDevice.h

0xc0170 · 2019-01-15T12:08:56Z

Note: feature-cellular-refactor will need a rebase from master, to resolve tools travis (fixed on master)

juhoeskeli · 2019-01-15T13:35:08Z

I am wondering is it possible to change the baudrate for e.g. MTB_ADV_WISE_1570 in mbed_app.json after this change goes through? From what I am hearing this value will be hardcoded in the future. I feel we should not limit ourselves to one value. It is because how the modem stack operates on this device that if one reads the data out too slowly incoming data will be lost. Thus in these situations I would like to still be able to configure the baudrate.

kjbracey · 2019-01-15T13:38:58Z

Thus in these situations I would like to still be able to configure the baudrate.

Why would you ever want to configure it slow? If it can go faster, then we can just make it fixed at that faster speed, I would have thought. Going faster should save power - reducing our active time.

There's usually no need to make internal bus speeds configurable.

(Not that setting the baud rate fast is sufficient to cover your modem problem, as I understand it)

Oh, and note that the speed is only fixed for devices where it's "onboard", which suggests we know everything about the wiring. If you're a plug-in module, speed can be set in the constructor or JSON as usual.

AriParkkila · 2019-01-15T14:21:42Z

@kjbracey-arm I copied some more docs from onboard_modem_api to here, and decreased trace levels from info to debug.

features/cellular/framework/API/CellularDevice.h

AriParkkila · 2019-01-15T14:26:01Z

@0xc0170 can I merge this branch to master - rebase seems quite tricky... Another option would be to close this feature branch and merge commits from my fork that is up to date with master https://github.com/AriParkkila/mbed-os/tree/rebase-feature-cellular-refactor

0xc0170 · 2019-01-16T10:45:24Z

@0xc0170 can I merge this branch to master

Which branch? There's another travis fix t hat is landing on master within a hour. To progress, either cherry pick it here to fix it or rebase the feature branch

AriParkkila · 2019-01-16T11:01:40Z

@0xc0170 this commit is going to feature-cellular-refactor branch. Can I use git merge (no rebase) to update the feature-branch up to master?

juhoeskeli · 2019-01-16T19:53:17Z

Thus in these situations I would like to still be able to configure the baudrate.

Why would you ever want to configure it slow? If it can go faster, then we can just make it fixed at that faster speed, I would have thought. Going faster should save power - reducing our active time.

Fast is good, I do prefer to have it that way. However, in this case the FW just comes with 9600 bps as default. At least in versions that we have available. Maybe that will change in the future FW version, who knows. I don't know how likely that is, but this is one example where configuration might be needed (unless the driver would be so smart as to "autosense" baudrate based on FW version or by trial&error).

There's usually no need to make internal bus speeds configurable.

(Not that setting the baud rate fast is sufficient to cover your modem problem, as I understand it)

You are correct it does not fix all problems. For some problems it seems to help though, which is why I configured it to 115200 instead of the factory default 9600. I was not able to download the 100kB+ file without doing that (and adjusting the amount of bytes read with one call to closer to the modem maximum).

Oh, and note that the speed is only fixed for devices where it's "onboard", which suggests we know everything about the wiring. If you're a plug-in module, speed can be set in the constructor or JSON as usual.

Noted, but I believe the device I am working with falls into that category.

Teppo Järvelin and others added 30 commits December 4, 2018 11:01

Cellular: Fixed CellularStateMachine to stop on all target states.

573b6ce

Cellular: Removed get_extended_signal_quality and changed get_signal_…

bc8515b

…quality

Removed CellularSIM interface.

176067b

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: eps ciot optimization network support check astyle fix

ec7d13e

Merge pull request ARMmbed#8891 from jarvte/remove_sim_interface

f52c9e3

Cellular: Remove sim interface

Merge pull request ARMmbed#8979 from TeemuKultala/cellular_ciot_eps_o…

60fe3c5

…n_feature_branch Cellular ciot eps on feature branch

Cellular: Move eDRX setting from power to network

1f744ef

Cellular: Move PSM setting from power to device

30bd353

Cellular: Move device_ready from power to device

ef2a091

Cellular: AT remove_urc_handler changed to set_urc_handler(prefix, 0)

6ec5f79

Cellular: Move ready_cb from power to device

7f7ffbe

Cellular: Unify set_at_mode and init_module into init()

09e2a7e

Cellular: Removed set_power_level()

5b2dc38

Cellular: Moved reset from power to device

65da29d

Cellular: Added shutdown()

d09ee0d

Cellular: changed support features to CellularProperty array.

0f1e08f

Merge pull request ARMmbed#8901 from AriParkkila/cell-refactor-power-api

a57bacf

Cell refactor power api

Merge pull request ARMmbed#9050 from jarvte/support_feature_to_key_value

c4138f4

Cellular: changed support features to CellularProperty array.

Cellular: Removed unnecessary checks after new

22d0d90

After this change we were able to change methods ATHandler::set_urc_handler and CellularDevice::set_ready_cb to void and simplify error handling.

Merge pull request ARMmbed#9090 from jarvte/remove_new_null_checks

104700c

Cellular: Removed unnecessary checks after new

Cellular: Change AT_CellularNetwork to use CellularProperties.

f76b21c

This change enables removing function has_registration from class AT_CellularNetwork and all targets inheriting AT_CellularNetwork.

Cellular: change stack_type_supported to get_property

40db17e

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: Add State machine unit tests.

e107599

Merge pull request ARMmbed#9120 from jarvte/create_generic_modem

f1691e4

Cellular: Add generic cellular modem

Cellular: added setting of data carrier support for UART.

e4956ce

Merge pull request ARMmbed#8971 from jarvte/add_datacarrier_detect

a7f4388

Cellular: added setting of data carrier support for UART.

ciarmcom requested a review from a team January 7, 2019 12:00

ciarmcom added the needs: review label Jan 7, 2019

jarvte approved these changes Jan 8, 2019

View reviewed changes

features/cellular/framework/targets/QUECTEL/UG96/mbed_lib.json Outdated Show resolved Hide resolved

0xc0170 added the release-version: feature branch label Jan 8, 2019

AnttiKauppila approved these changes Jan 10, 2019

View reviewed changes

0xc0170 approved these changes Jan 14, 2019

View reviewed changes

kjbracey suggested changes Jan 14, 2019

View reviewed changes

Ari Parkkila added 2 commits January 15, 2019 02:27

Cellular: Default modem drivers with FF_ARDUINO

d3b4caf

Cellular: Power API updated to match onboard_modem_api

84216a5

kjbracey reviewed Jan 15, 2019

View reviewed changes

0xc0170 added needs: work and removed needs: review labels Jan 15, 2019

kjbracey reviewed Jan 15, 2019

View reviewed changes

features/cellular/framework/API/CellularDevice.h Outdated Show resolved Hide resolved

Cellular: Update API description to match better onboard_modem_api

a42aa44

AriParkkila force-pushed the cell-get-default-instance branch from 990ca98 to a42aa44 Compare January 16, 2019 06:40

kjbracey approved these changes Jan 16, 2019

View reviewed changes

0xc0170 force-pushed the feature-cellular-refactor branch from 6b0639d to 007caa0 Compare January 22, 2019 10:40

AriParkkila closed this Jan 22, 2019

studavekar removed the needs: work label Jan 22, 2019

mirelachirica mentioned this pull request Jan 29, 2019

Cellular: Added API for setting default parameters to a network inter… #9287

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cellular refactor power and get_default_instance, also remove deprecated #9273

Cellular refactor power and get_default_instance, also remove deprecated #9273

AriParkkila commented Jan 7, 2019 •

edited

Loading

ciarmcom commented Jan 7, 2019

kjbracey left a comment

AriParkkila commented Jan 15, 2019

kjbracey left a comment

0xc0170 commented Jan 15, 2019

juhoeskeli commented Jan 15, 2019

kjbracey commented Jan 15, 2019 •

edited

Loading

AriParkkila commented Jan 15, 2019

AriParkkila commented Jan 15, 2019

0xc0170 commented Jan 16, 2019

AriParkkila commented Jan 16, 2019

juhoeskeli commented Jan 16, 2019

Cellular refactor power and get_default_instance, also remove deprecated #9273

Cellular refactor power and get_default_instance, also remove deprecated #9273

Conversation

AriParkkila commented Jan 7, 2019 • edited Loading

Description

Pull request type

Reviewers

ciarmcom commented Jan 7, 2019

kjbracey left a comment

Choose a reason for hiding this comment

AriParkkila commented Jan 15, 2019

kjbracey left a comment

Choose a reason for hiding this comment

0xc0170 commented Jan 15, 2019

juhoeskeli commented Jan 15, 2019

kjbracey commented Jan 15, 2019 • edited Loading

AriParkkila commented Jan 15, 2019

AriParkkila commented Jan 15, 2019

0xc0170 commented Jan 16, 2019

AriParkkila commented Jan 16, 2019

juhoeskeli commented Jan 16, 2019

AriParkkila commented Jan 7, 2019 •

edited

Loading

kjbracey commented Jan 15, 2019 •

edited

Loading