Add MTS dragonfly, MTS dragonfly l471 #7304

cedrickkukela-cd · 2018-06-23T01:03:23Z

Description

Adding Target for Multitech Dragonfly L471QG.
#7177

Pull request type

[  ] Fix
[ ] Refactor
[ X ] New target
[ ] Feature
[ ] Breaking change

cedrickkukela-cd · 2018-06-23T01:04:29Z

adding test results

IARresult3.txt
GCCresults2.txt
ARM_results.txt
ARM6results2.txt

cedrickkukela-cd · 2018-06-23T01:04:39Z

adding @maclobdell

maclobdell · 2018-06-25T14:20:27Z

Hi Cedrick - can you confirm why the two pull request and not one? Updating commits on an existing pull request will work if you want to combine the pull requests. It is easier on our maintainers and test system if there is just one pull request to add a new target. Please let me know what you prefer.

cedrickkukela-cd · 2018-06-25T14:35:23Z

They are not the same, but to pass all the test requires both. This one includes only the changes done under feature. Will other are changes done under target. If merging the two would make things easier let me know.

maclobdell · 2018-06-25T21:38:02Z

@cedrickkukela-cd - It looks like all the commits for #7177 may have come along for the ride which makes things a bit confusing. Can you make this PR just include the changes (external to the target directory) that you need to enable the cellular interface? This PR can wait for the other PR to get in. If this is all too confusing and a headache, let me know and maybe PR 7177 can be closed in favor of this one.

cedrickkukela-cd · 2018-06-26T11:54:53Z

if it is easier to just use 7177 and it wont delay things. lets just go with that one.

maclobdell · 2018-06-28T03:19:17Z

@cedrickkukela-cd - Just to confirm for everyone - based on our conversation, this will be the only pull request that will add support for Dragonfly Nano. Looks like there are still a few outstanding test failures. We'll get more reviewers involved and offer suggestions. In the meantime, please continue your work to isolate and resolve the remaining issues.

AnttiKauppila · 2018-06-28T06:36:07Z

@jarvte and @AriParkkila Please review this

AriParkkila

It's easy to follow Mbed OS coding conventions, just apply AStyle.exe -n --options=.astylerc filename.h/cpp in mbed-os root-folder.

AriParkkila · 2018-06-28T07:11:36Z

targets/targets.json

+            }
+        },
+        "detect_code": ["0312"],
+        "macros_add": [ "RTC_LSE=1", "CELLULAR_DEVICE=UBLOX_PPP"],


CELLULAR_DEVICE need not be defined here, that's defined in CellularTargets.h

This has not been answered, still valid?

Yes, remove from here.

RTC_LSE is no more used in TARGET_STM

I will be pushing a commit soon. I removed RTC_LSE=1. In CellularTargets.h, I see that CELLULAR_DEVICE is being defined as UBLOX_PPP. However, if I remove "CELLULAR_DEVICE=UBLOX_PPP" from targets.json, it fails to compile at least some tests.

AriParkkila · 2018-06-28T07:18:14Z

features/cellular/framework/AT/AT_CellularNetwork.cpp

@@ -345,8 +345,14 @@ nsapi_error_t AT_CellularNetwork::open_data_channel()
 {
 #if NSAPI_PPP_AVAILABLE
    tr_info("Open data channel in PPP mode");
+
+#ifdef TARGET_MTS_DRAGONFLY_L471QG


AT+CGDATA is likely supported. Can you check/share AT manual of the cellular module?

https://www.u-blox.com/sites/default/files/SARA-R4-SARA-N4_ATCommands_%28UBX-17003787%29.pdf

AT+CGDATA is not supported on the sara r410 with the currently approved firmware. This was confirmed with a ublox fae.

I ran the astyle utility on the two file mentioned and above and quite a few lines changed when i diffed. Is this expected.

@cedrickkukela-cd Features are not yet astyled. We will do it soon. Please, just use it on new files in this pull request (note, targets folder is being ignored at the moment, but would be nice if they follow as well - targets HAL implementation).

@cedrickkukela-cd should this be dynamic? The reason is that maybe TARGET_MTS_DRAGONFLY_L471QG could hold different kind of cellular modules (in the future) and some of those modules might support AT+CGDATA?

For dynamic implementation you could use unsupported flagging, just define AT_CellularBase::SupportedFeature with e.g. flag name AT_CGDATA in you CellularDevice and and then in open_data_channel if (!is_supported(AT_CGDATA)).

@cedrickkukela-cd Any update for the question regarding being dynamic?

Dynamic would be better then there is no need for extra #ifdef's. You can check how it was done with QUECTEL_BG96.cpp and how it was used in AT_CellularInformation::get_serial_number(..

@cedrickkukela-cd please see an an example how you could do this at
AriParkkila@c7bd3cb

maclobdell · 2018-07-05T20:46:17Z

I just ran all tests (except cellular and network socket) against the branch referenced in this PR on a new Dragonflay Nano (B01). All passed for IAR, ARM, & GCC_ARM compilers.
Dragonfly_Nano_PR7304_MbedOS_Tests_B01_GCC_ARM_5July2018.txt
Dragonfly_Nano_PR7304_MbedOS_Tests_B01_ARM_6July2018.txt
Dragonfly_nano_PR7304_MbedOS_Tests_IAR_B01_5July2018.txt

maclobdell · 2018-07-10T18:55:59Z

@cedrickkukela-cd - did you use a json config file to configure and enable Cellular with PPP mode? If so, can you share? Or share any details on how you ran cellular and network socket tests. This will be helpful.

cedrickkukela-cd · 2018-07-10T19:01:16Z

sure
here is the mbed_app i used.

Did you get any further with shield testing?

mbed_app.json.txt

maclobdell · 2018-07-23T15:15:18Z

@0xc0170 - what is the status of the review? From a basic Mbed-OS point of view, I think everything looks good. We are working through an issue with the cellular network connectivity. While we finish that up, can this PR be merged?

cmonr · 2018-07-23T15:27:00Z

@maclobdell From the conversation, it looked like there were still changes being made. Thanks for the FYI. We'll take a look at this and puti t through CI soon.

maclobdell · 2018-08-06T13:15:50Z

@cmonr - please urgently help to complete reviews and testing for this PR. Please tag it appropriately. We believe it is ready to go.

0xc0170 · 2018-08-06T13:17:51Z

targets/targets.json

+            }
+        },
+        "detect_code": ["0312"],
+        "macros_add": [ "RTC_LSE=1", "CELLULAR_DEVICE=UBLOX_PPP"],


This has not been answered, still valid?

0xc0170 · 2018-08-06T13:18:17Z

targets/targets.json

+        "release_versions": ["2", "5"],
+        "device_name": "STM32L471QG",
+        "bootloader_supported": true,
+        "features": ["IPV4", "NANOSTACK", "COMMON_PAL"]


nanostack not needed anymore as feature, same should be true for common_pal

true, common_pal not needed anymore.

0xc0170 · 2018-08-06T13:18:39Z

targets/TARGET_STM/TARGET_STM32L4/TARGET_MTS_DRAGONFLY_L471QG/ublox_low_level_api.c

+    gpio_init_inout(&gpio, MDMPWRON,  PIN_OUTPUT, OpenDrainNoPull, 1);
+#endif
+    gpio_init_out_ex(&gpio, MDMRTS,    0);
+    //gpio_init_in_ex(&gpio,  MDMCURRENTSENSE, PullNone);


remove this dead code?

0xc0170 · 2018-08-06T13:19:25Z

features/cellular/framework/AT/AT_CellularNetwork.cpp

@@ -345,8 +345,14 @@ nsapi_error_t AT_CellularNetwork::open_data_channel()
 {
 #if NSAPI_PPP_AVAILABLE
    tr_info("Open data channel in PPP mode");
+
+#ifdef TARGET_MTS_DRAGONFLY_L471QG


@cedrickkukela-cd Any update for the question regarding being dynamic?

0xc0170 · 2018-08-06T13:19:50Z

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

@@ -34,7 +34,11 @@ bool UBLOX_PPP_CellularNetwork::get_modem_stack_type(nsapi_ip_stack_t requested_

 bool UBLOX_PPP_CellularNetwork::has_registration(RegistrationType reg_type)
 {
-    return (reg_type == C_REG || reg_type == C_GREG);
+#ifdef TARGET_DRAGONFLY_L471QG
+	return (reg_type == C_REG || reg_type == C_EREG);


misaligned (2 spaces more here or tab used?) ?

0xc0170 · 2018-08-06T13:20:57Z

@cmonr - please urgently help to complete reviews and testing for this PR. Please tag it appropriately. We believe it is ready to go.

👍 I left only few questions.

@ARMmbed/mbed-os-wan Please review this target addition. @AriParkkila left few comments but was not certain if this was requesting changes or approved with minor comments?

AnttiKauppila · 2018-08-06T14:17:22Z

@jarvte Can you review from Cellular perspective as Ari is on holiday?

jarvte · 2018-08-07T10:17:53Z

targets/targets.json

+            }
+        },
+        "detect_code": ["0312"],
+        "macros_add": [ "RTC_LSE=1", "CELLULAR_DEVICE=UBLOX_PPP"],


Yes, remove from here.

jarvte · 2018-08-07T10:18:41Z

targets/targets.json

+        "release_versions": ["2", "5"],
+        "device_name": "STM32L471QG",
+        "bootloader_supported": true,
+        "features": ["IPV4", "NANOSTACK", "COMMON_PAL"]


true, common_pal not needed anymore.

jarvte · 2018-08-07T10:28:12Z

features/cellular/framework/AT/AT_CellularNetwork.cpp

@@ -345,8 +345,14 @@ nsapi_error_t AT_CellularNetwork::open_data_channel()
 {
 #if NSAPI_PPP_AVAILABLE
    tr_info("Open data channel in PPP mode");
+
+#ifdef TARGET_MTS_DRAGONFLY_L471QG


Dynamic would be better then there is no need for extra #ifdef's. You can check how it was done with QUECTEL_BG96.cpp and how it was used in AT_CellularInformation::get_serial_number(..

maclobdell · 2018-08-17T18:42:20Z

@cedrickkukela-cd @felser Please evaluate the review comments on this PR. Let's work closely to get this merged.

cmonr · 2018-08-24T05:24:50Z

@cedrickkukela-cd @maclobdell Have there been any updates?

0xc0170

Just one git file edit

0xc0170 · 2018-10-01T07:49:15Z

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

@@ -34,7 +34,15 @@ bool UBLOX_PPP_CellularNetwork::get_modem_stack_type(nsapi_ip_stack_t requested_

 AT_CellularNetwork::RegistrationMode UBLOX_PPP_CellularNetwork::has_registration(RegistrationType reg_type)
 {
+//<<<<<<< HEAD


This is git edit here (can you clean up this file ( the commenst should not be added here)

I pushed up this change.

0xc0170 · 2018-10-02T09:20:55Z

...s/TARGET_STM/TARGET_STM32L4/TARGET_MTS_DRAGONFLY_L471QG/device/TOOLCHAIN_IAR/stm32l471xx.icf

+define region SRAM1_region = mem:[from __region_SRAM1_start__ to __region_SRAM1_end__];
+
+/* Stack complete SRAM2 and Heap 1/3 of SRAM1 */
+define symbol __size_cstack__ = 0x7e00;


There's an effort to bring this to 1k (please update)

@0xc0170 - are you talking about the default stack or heap size? Do you mean 1K size or 1K alignment? For IAR, in order to get the tests passing, the heap must be explicitly set to a value with enough room to support the networking stack. Is this a blocking issue? If not, please approve so MultiTech can meet their deadline of releasing this code.

__size_cstack__ stack size - to be aligned with #7238 - provides reasons for the size

Stack decreased to 0x400/1KB.

0xc0170

Just that stack for IAR and the rest LGTM

maclobdell · 2018-10-03T16:15:13Z

@0xc0170 - please confirm that the latest change addresses your feedback and this PR is ready to merge.

jarvte · 2018-10-04T04:13:13Z

@cmonr my comments have been fixed

maclobdell · 2018-10-05T12:52:26Z

@0xc0170 , @cmonr - please wrap up the review and merge. this PR is urgent to get completed.

0xc0170 · 2018-10-05T12:58:36Z

@cmonr my comments have been fixed

Leave review feedback (approve - to dismiss the previous one). I'll dismiss it

Seems I cant as it was changed :)

0xc0170 · 2018-10-05T13:00:35Z

/morph build

mbed-ci · 2018-10-05T14:12:42Z

Build : SUCCESS

Build number : 3251
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7304/

Triggering tests

/morph test
/morph mbed2-build

0xc0170 · 2018-10-05T17:54:03Z

/morph export-build

0xc0170 · 2018-10-05T17:54:09Z

/morph mbed2-build

mbed-ci · 2018-10-05T21:37:58Z

Exporter Build : SUCCESS

Build number : 2845
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7304/

mbed-ci · 2018-10-06T06:10:50Z

Test : SUCCESS

Build number : 3058
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7304/3058

adbridge · 2018-10-08T13:14:48Z

This PR contains conflicting changes with #7969 (which is scheduled for 5.11), thus pushing this out also.

maclobdell · 2018-10-08T16:41:39Z

Hi Anna - This PR is critical to get into 5.10.1. It cannot wait any longer. There is a product release depending on this PR.

adbridge · 2018-10-08T16:59:02Z

@maclobdell Please talk to @ChiefBureaucraticOfficer

cedrickkukela-cd mentioned this pull request Jun 23, 2018

adding TARGET MTS_DRAGONFLY_L471QG #7177

Closed

NirSonnenschein added the needs: review label Jun 24, 2018

0xc0170 added needs: work and removed needs: review labels Jun 26, 2018

cedrickkukela-cd changed the title ~~Mts dragonfly l471 qg pull request feature changes~~ adding targes mts dragonfly Mts dragonfly l471and adding cell support Jun 27, 2018

AriParkkila reviewed Jun 28, 2018

View reviewed changes

AriParkkila mentioned this pull request Aug 2, 2018

PPPCellularInterface::connect calls nwk_registration with a hard coded value #7213

Closed

0xc0170 requested changes Aug 6, 2018

View reviewed changes

0xc0170 requested a review from a team August 6, 2018 13:21

jarvte suggested changes Aug 7, 2018

View reviewed changes

0xc0170 changed the title ~~adding targes mts dragonfly Mts dragonfly l471and adding cell support~~ Add MTS dragonfly, MTS dragonfly l471 Aug 22, 2018

AriParkkila mentioned this pull request Aug 23, 2018

Cellular: Support for GPRS dial-up #7870

Merged

0xc0170 requested changes Oct 1, 2018

View reviewed changes

Delete commented out merge conflict resolution

f0a5dce

0xc0170 reviewed Oct 2, 2018

View reviewed changes

0xc0170 requested changes Oct 2, 2018

View reviewed changes

Configure IAR stack size to 1KB

9f9a858

0xc0170 approved these changes Oct 5, 2018

View reviewed changes

0xc0170 added needs: CI release-version: 5.10.1 and removed needs: work labels Oct 5, 2018

NirSonnenschein added ready for merge and removed needs: CI labels Oct 6, 2018

0xc0170 merged commit 365d61e into ARMmbed:master Oct 8, 2018

0xc0170 removed the ready for merge label Oct 8, 2018

AriParkkila mentioned this pull request Oct 8, 2018

Cellular: Fix Greentea cellular power test #8236

Closed

adbridge added release-version: 5.11.0-rc1 and removed release-version: 5.10.1 labels Oct 8, 2018

AnttiKauppila mentioned this pull request Oct 11, 2018

Mts dragonfly l471 qg #8382

Closed

Add MTS dragonfly, MTS dragonfly l471 #7304

Add MTS dragonfly, MTS dragonfly l471 #7304

Uh oh!

Conversation

cedrickkukela-cd commented Jun 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Uh oh!

cedrickkukela-cd commented Jun 23, 2018

Uh oh!

cedrickkukela-cd commented Jun 23, 2018

Uh oh!

maclobdell commented Jun 25, 2018

Uh oh!

cedrickkukela-cd commented Jun 25, 2018

Uh oh!

maclobdell commented Jun 25, 2018

Uh oh!

cedrickkukela-cd commented Jun 26, 2018

Uh oh!

maclobdell commented Jun 28, 2018

Uh oh!

AnttiKauppila commented Jun 28, 2018

Uh oh!

AriParkkila left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maclobdell commented Jul 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maclobdell commented Jul 10, 2018

Uh oh!

cedrickkukela-cd commented Jul 10, 2018

Uh oh!

maclobdell commented Jul 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmonr commented Jul 23, 2018

Uh oh!

maclobdell commented Aug 6, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cedrickkukela-cd commented Jun 23, 2018 •

edited

Loading

maclobdell commented Jul 5, 2018 •

edited

Loading

maclobdell commented Jul 23, 2018 •

edited

Loading

maclobdell Oct 2, 2018 •

edited

Loading

0xc0170 Oct 2, 2018 •

edited

Loading