Remove target dependency in ublox cellular APIs #11580

wajahat-abbas · 2019-09-27T07:36:35Z

Description

The intention of this PR is to allow ublox cellular APIs to be used with custom targets.
When using ublox modem with a different MCU, user only has to define correct modem macros in their mbed_app.json like this:
"K64F": { "target.macros_add": ["UBX_MDM_SARA_R41XM", "UBX_MDM_SARA_R412M"] }

Previously, as discussed in #11505 #11170, the APIs were tied to ublox mbed boards so it was troublesome to use the with a different MCU or a custom board.

Pull request type

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

Reviewers

@fahim-ublox @RobMeades @mudassar-ublox @pilotak

Release Notes

pilotak · 2019-09-27T08:13:47Z

features/cellular/framework/targets/UBLOX/AT/UBLOX_AT.cpp

@@ -98,7 +104,7 @@ nsapi_error_t UBLOX_AT::init()

    nsapi_error_t err = NSAPI_ERROR_OK;

-#ifdef TARGET_UBLOX_C027
+#ifdef UBX_MDM_SARA_G350


shouldn't this be UBX_MDM_SARA_G3XX one could have G340. It would than lead to UBX_MDM_SARA_G350 not being used therefore UBX_MDM_SARA_G3XX would be enough to define

Let me double check the differences and i will fix this.

ciarmcom · 2019-09-27T09:00:25Z

@wajahat-ublox, thank you for your changes.
@ARMmbed/mbed-os-wan @ARMmbed/mbed-os-maintainers please review.

0xc0170

Introducing the new macros, where they are documented ? UBX_MDM_SARA_U2XX vs UBX_MDM_SARA_U211 ?

Wouldn't be easier to just introduce N211 macro and get U2XX automatically (cellular target header for deduction of the common code) ?

wajahat-abbas · 2019-09-30T10:02:56Z

@0xc0170
Do you mean that e.g. only UBX_MDM_SARA_U201 macro is set in json file and we set the UBX_MDM_SARA_U2XX macro from a header file?
The reason for doing this is that in a modem family e.g. SARA-G3XX there is a G350 as well as G340 modem. The Ublox mbed board C027 comes with G350 however a user might want to use G340 with this own custom board. In this case, user simply has to set the family macro UBX_MDM_SARA_G3XX and be able to use the cellular APIs. If we set the family macros from header file based upon modem macro then in the case mentioned above UBX_MDM_SARA_G350 modem macro will not be set hence UBX_MDM_SARA_G3XX family macro also won't be set.

Also please elaborate where am i supposed to document these new macros. Is this supposed to go in Cellular readme or is there some other section for this?

0xc0170 · 2019-09-30T10:08:40Z

I was just wondering if we can simplify this somehow. Setting two macros for almost the same thing (if we can just use the modem and the family can be found in the header file (although it would mean to maintain the header file to get proper family). So might be after all the same.

Documentation - what types are there and what modems are supported - is it documented anywhere or do I need to scan the sources to find out macros U2XX, etc ?

wajahat-abbas · 2019-09-30T10:22:38Z

@0xc0170
I get your point on documenting it but apart from PR please let me know where shall i add the details? few places i can think of are source, json file, cellular docs. What do you recommend?

0xc0170 · 2019-09-30T10:54:28Z

@ARMmbed/mbed-os-wan any better place to document the modem hierarchy than readme in the targets folder in cellular?

adbridge · 2019-10-07T14:01:17Z

@ARMmbed/mbed-os-wan could you please answer @0xc0170 's question...

AnttiKauppila · 2019-10-08T06:50:37Z

targets/UBLOX folder can have it's own readme.md, that is the most logical place to find information regarding to UBLOX modems.

wajahat-abbas · 2019-10-08T10:27:58Z

@0xc0170 Readme file added. Please review.

0xc0170 · 2019-10-18T13:58:11Z

I am reviewing now

Conflicting files: features/cellular/framework/targets/UBLOX/AT/UBLOX_AT_CellularStack.cpp

@wajahat-ublox Can you rebase to resolve conflict, will start CI

0xc0170 · 2019-10-18T13:58:39Z

@ARMmbed/mbed-os-wan Please review

wajahat-abbas · 2019-10-21T06:21:07Z

@0xc0170 Conflict resolved and rebased to master.

0xc0170 · 2019-10-21T08:17:58Z

CI started

mbed-ci · 2019-10-21T10:14:12Z

Test run: FAILED

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

Failed test jobs:

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

wajahat-abbas · 2019-10-21T10:50:11Z

@0xc0170 I looked at the CI logs, any idea why it is building for UBLOX_C030_R41XM? I mean actually there are two targets, UBLOX_C030_R410M and UBLOX_C030_R412M. My assumption is that UBLOX_C030_R41XM is only used for target inheritance purpose.

0xc0170 · 2019-10-21T11:23:33Z

Shouldn't that target have this : "public": false set ? That would avoid building

wajahat-abbas · 2019-11-05T05:08:48Z

ill restart CI asap

@0xc0170 Done. Please have a look.

0xc0170 · 2019-11-05T08:05:11Z

@wajahat-ublox Can you please rebase instead of merge ? The last commit should not be on your branch.

0xc0170 · 2019-11-05T11:53:33Z

CI restarted

mbed-ci · 2019-11-05T12:50:11Z

Test run: SUCCESS

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

0xc0170 · 2019-11-05T13:00:32Z

features/cellular/framework/targets/UBLOX/PPP/UBLOX_PPP.cpp

@@ -57,6 +61,24 @@ static const intptr_t cellular_properties[AT_CellularBase::PROPERTY_MAX] = {
    0,  // PROPERTY_NON_IP_PDP_TYPE
    1,  // PROPERTY_AT_CGEREP
 };
+#else
+static const intptr_t cellular_properties[AT_CellularBase::PROPERTY_MAX] = {


@AnttiKauppila Is this approach fine ?

@wajahat-ublox Do we need to have this cellular_properties defined always?

@0xc0170 The else condition is there for a cellular program using UBLOX APIs with an unsupported modem. Yes in case of non cellular programs, it should not be part of final binary (optimized out).

0xc0170 · 2019-11-05T13:01:15Z

Last question about else condition for properties, as they are always defined even-though linker should be able to eliminate it

0xc0170 · 2019-11-13T17:04:18Z

CI started (last run is few days old)

mbed-ci · 2019-11-13T20:53:21Z

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

0xc0170 · 2019-11-14T09:49:17Z

Test failed because of #11862, we will investigate

adbridge · 2019-11-15T12:30:34Z

CI restarted

mbed-ci · 2019-11-15T13:53:54Z

Test run: SUCCESS

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

wajahat-abbas changed the title ~~Remove target dependancy in ublox cellular APIs~~ Remove target dependency in ublox cellular APIs Sep 27, 2019

pilotak reviewed Sep 27, 2019

View reviewed changes

ciarmcom requested review from a team September 27, 2019 09:00

ciarmcom added the needs: review label Sep 27, 2019

0xc0170 requested changes Sep 30, 2019

View reviewed changes

wajahat-abbas force-pushed the ubx_modems_custom_boards branch from 9f3354c to 2f5706b Compare September 30, 2019 10:04

wajahat-abbas requested a review from 0xc0170 October 8, 2019 10:27

0xc0170 added the needs: work label Oct 18, 2019

0xc0170 approved these changes Oct 18, 2019

View reviewed changes

0xc0170 removed the needs: work label Oct 18, 2019

wajahat-abbas force-pushed the ubx_modems_custom_boards branch from 9eb0c9b to ed95989 Compare October 21, 2019 06:18

0xc0170 added needs: CI and removed needs: review labels Oct 21, 2019

0xc0170 removed the needs: CI label Oct 21, 2019

0xc0170 added the needs: work label Nov 4, 2019

Wajahat Abbas added 2 commits November 5, 2019 14:25

Remove target dependency in ublox cellular APIs

a5015c2

Removed error directives

98f3baa

wajahat-abbas force-pushed the ubx_modems_custom_boards branch from 0f12c68 to 98f3baa Compare November 5, 2019 09:40

wajahat-abbas requested a review from 0xc0170 November 5, 2019 11:52

0xc0170 approved these changes Nov 5, 2019

View reviewed changes

0xc0170 added needs: CI and removed needs: work labels Nov 5, 2019

0xc0170 reviewed Nov 5, 2019

View reviewed changes

0xc0170 added needs: review and removed needs: CI labels Nov 5, 2019

wajahat-abbas requested review from AnttiKauppila and 0xc0170 November 8, 2019 06:21

0xc0170 approved these changes Nov 13, 2019

View reviewed changes

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

0xc0170 added ready for merge and removed needs: CI labels Nov 15, 2019

0xc0170 merged commit 7b79157 into ARMmbed:master Nov 15, 2019

0xc0170 removed the ready for merge label Nov 15, 2019

Remove target dependency in ublox cellular APIs #11580

Remove target dependency in ublox cellular APIs #11580

Uh oh!

Conversation

wajahat-abbas commented Sep 27, 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!

pilotak Sep 27, 2019

Choose a reason for hiding this comment

Uh oh!

wajahat-abbas Sep 27, 2019

Choose a reason for hiding this comment

Uh oh!

ciarmcom commented Sep 27, 2019

Uh oh!

0xc0170 left a comment

Choose a reason for hiding this comment

Uh oh!

wajahat-abbas commented Sep 30, 2019

Uh oh!

0xc0170 commented Sep 30, 2019

Uh oh!

wajahat-abbas commented Sep 30, 2019

Uh oh!

0xc0170 commented Sep 30, 2019

Uh oh!

adbridge commented Oct 7, 2019

Uh oh!

AnttiKauppila commented Oct 8, 2019

Uh oh!

wajahat-abbas commented Oct 8, 2019

Uh oh!

0xc0170 commented Oct 18, 2019

Uh oh!

0xc0170 commented Oct 18, 2019

Uh oh!

wajahat-abbas commented Oct 21, 2019

Uh oh!

0xc0170 commented Oct 21, 2019

Uh oh!

mbed-ci commented Oct 21, 2019

Test run: FAILED

Uh oh!

wajahat-abbas commented Oct 21, 2019

Uh oh!

0xc0170 commented Oct 21, 2019

Uh oh!

wajahat-abbas commented Nov 5, 2019

Uh oh!

0xc0170 commented Nov 5, 2019

Uh oh!

0xc0170 commented Nov 5, 2019

Uh oh!

mbed-ci commented Nov 5, 2019

Test run: SUCCESS

Uh oh!

0xc0170 Nov 5, 2019

Choose a reason for hiding this comment

Uh oh!

wajahat-abbas Nov 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Nov 5, 2019

Uh oh!

0xc0170 commented Nov 13, 2019

Uh oh!

mbed-ci commented Nov 13, 2019

Test run: FAILED

Uh oh!

0xc0170 commented Nov 14, 2019

Uh oh!

adbridge commented Nov 15, 2019

Uh oh!

mbed-ci commented Nov 15, 2019

Test run: SUCCESS

wajahat-abbas commented Sep 27, 2019 •

edited

Loading

wajahat-abbas Nov 6, 2019 •

edited

Loading