Remove Cellular dependency in netsocket #13809

jeromecoutant · 2020-10-23T14:55:45Z

Summary of changes

Proposition is to be able to remove Cellular code in mbed-os

How to reproduce:
Create a .mbedignore file with

mbed-os/connectivity/cellular*
mbed-os/connectivity/drivers/cellular*

Impact of changes

Migration actions required

Documentation

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

ciarmcom · 2020-10-23T15:00:30Z

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

pan- · 2020-10-23T16:33:31Z

Thanks @jeromecoutant I don't think you need the .mbedignore file if you don't add cellular to the require clause in mbed_app.json .

The PR is a good starting point but I believe more work is required to pass it through. If cellular is not present then CellularInterface.h, CellularNonIPSocket.h and ControlPlane_netif.h should not return any content when included.

0xc0170

This shall work also with CMake (although we will need to remove linking netsockets to cellular as it was dependent before this patch).
It should be other way around, if cellular is selected, netsockets would get in as well.

Thanks @jeromecoutant I don't think you need the .mbedignore file if you don't add cellular to the require clause in mbed_app.json .

I assume this is for not using baremetal, but Mbed OS with ignoring some parts - an example: using networking via wire but not cellular.

0xc0170 · 2020-10-26T11:39:30Z

Set as needs: work, to verify the files mentioned above.

jeromecoutant · 2020-11-02T16:45:02Z

Set as needs: work, to verify the files mentioned above.

I think there is no risk to accept this PR as it is.
Up to you to go further in some other PR ?

Thx

pan- · 2020-11-04T09:23:18Z

@jeromecoutant This PR doesn't work with ARMC6:

[Error] @0,0: L6218E: Undefined symbol CellularInterface::get_target_default_instance() (referred from BUILD/NRF52840_DK/ARMC6/mbed-os/connectivity/netsocket/source/NetworkInterfaceDefaults.o).

jeromecoutant · 2020-11-04T16:16:26Z

@pan-

This PR doesn't work with ARMC6:

Can you explicit your use case ?
For me, it seems OK:

mbed test -t ARM -m NRF52840_DK -v -n connectivity-*
Build successes:
  * NRF52840_DK::ARMC6::CONNECTIVITY-FEATURE_BLE-SOURCE-CORDIO-TESTS-CORDIO_HCI-DRIVER
  * NRF52840_DK::ARMC6::CONNECTIVITY-MBEDTLS-TESTS-TESTS-MBEDTLS-MULTI
  * NRF52840_DK::ARMC6::CONNECTIVITY-MBEDTLS-TESTS-TESTS-MBEDTLS-SELFTEST
  * NRF52840_DK::ARMC6::CONNECTIVITY-NETSOCKET-TESTS-TESTS-NETWORK-L3IP
  * NRF52840_DK::ARMC6::MBED-BUILD

pan- · 2020-11-04T16:45:53Z

@jeromecoutant Here are the steps to reproduce it:

Import mbed os blinky example
Add a .mbedignore file that excludes mbed-os/connectivity/cellular* and mbed-os/connectivity/drivers/cellular*
Compile

jeromecoutant · 2020-11-04T17:45:33Z

OK, patch improved, please start CI

Pull request has been modified.

jeromecoutant · 2020-11-09T12:06:40Z

Patch needed for #13598 workaround

0xc0170

Do we need that many if PRESENT in NEtworkInterfaceDefaults? I wonder if we can gather methods together and have just one (so it is not spreaded all over the file ) ?

This should help also CMake as well as it uses the same _PRESENT for components (backward compatibility),

0xc0170 · 2020-11-09T14:41:18Z

connectivity/netsocket/tests/TESTS/netsocket/udp/udpsocket_echotest_burst.cpp

 #include "CellularDevice.h"
+#endif // MBED_CONF_CELLULAR_PRESENT


the comment states something different, is this app baudrate rather than cellular present?

Just added the same ifdef as it is already done bellow in this file

jeromecoutant · 2020-11-13T14:51:05Z

Let's start CI ?

jeromecoutant · 2020-11-23T13:36:12Z

ping

0xc0170 · 2020-11-23T13:43:58Z

CI started

I requested connectivity team to review

Tip: Create a .mbedignore file with connectivity/cellular* connectivity/drivers/cellular*

jeromecoutant · 2020-12-03T17:17:39Z

Please check

Done

0xc0170 · 2020-12-09T16:28:03Z

CI started

mbed-ci · 2020-12-09T18:21:59Z

Jenkins CI Test : ❌ FAILED

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	❌
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM	✔️

0xc0170 · 2020-12-10T09:45:23Z

@jeromecoutant unittests require update. see the failures in the logs


[2020-12-09T16:43:26.709Z] /builds/workspace/mbed-os-ci_unittests@2/unitTest-7245/mbed-os/connectivity/netsocket/tests/UNITTESTS/netsocket/CellularNonIPSocket/test_CellularNonIPSocket.cpp:43: undefined reference to `mbed::CellularNonIPSocket::CellularNonIPSocket()'

Pull request has been modified.

jeromecoutant · 2020-12-17T14:38:09Z

unittests require update. see the failures in the logs

Done, Thx @0xc0170

mbed-ci · 2020-12-17T15:39:59Z

Jenkins CI Test : ✔️ SUCCESS

Build Number: 3 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_cmake-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-test	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️

ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Oct 23, 2020

ciarmcom requested a review from a team October 23, 2020 15:00

ciarmcom added the needs: review label Oct 23, 2020

0xc0170 previously approved these changes Oct 26, 2020

View reviewed changes

mergify bot added needs: CI and removed needs: review labels Oct 26, 2020

0xc0170 requested a review from a team October 26, 2020 11:36

0xc0170 added needs: work and removed needs: CI labels Oct 26, 2020

jeromecoutant mentioned this pull request Nov 2, 2020

Avoid compiling features/cellular files when not needed #7457

Closed

jeromecoutant force-pushed the PR_CELLULAR branch from 2d32406 to 5b3ec2c Compare November 4, 2020 17:45

jeromecoutant mentioned this pull request Nov 9, 2020

Error while running greentea on DISCO_L475VG_IOT01A ARMmbed/mbed-os-tools#245

Closed

0xc0170 reviewed Nov 9, 2020

View reviewed changes

0xc0170 requested review from a team and 0xc0170 November 23, 2020 13:42

0xc0170 added needs: review and removed needs: work labels Nov 23, 2020

0xc0170 added needs: work and removed needs: CI labels Nov 30, 2020

Remove Cellular dependency in netsocket

8e58d64

Tip: Create a .mbedignore file with connectivity/cellular* connectivity/drivers/cellular*

jeromecoutant force-pushed the PR_CELLULAR branch from 08dd6ff to 8e58d64 Compare December 3, 2020 17:16

0xc0170 requested a review from pan- December 9, 2020 16:25

0xc0170 added needs: review and removed needs: work labels Dec 9, 2020

0xc0170 previously approved these changes Dec 9, 2020

View reviewed changes

mergify bot added needs: CI and removed needs: review labels Dec 9, 2020

pan- previously approved these changes Dec 9, 2020

View reviewed changes

Cellular unittests update

9eb86a2

0xc0170 approved these changes Dec 17, 2020

View reviewed changes

0xc0170 added ready for merge and removed needs: CI labels Dec 17, 2020

0xc0170 merged commit 9315f05 into ARMmbed:master Dec 17, 2020

mergify bot removed the ready for merge label Dec 17, 2020

mbedmain added release-version: 6.7.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Jan 25, 2021

jeromecoutant deleted the PR_CELLULAR branch April 23, 2021 11:56

		#include "CellularDevice.h"
		#endif // MBED_CONF_CELLULAR_PRESENT

Remove Cellular dependency in netsocket #13809

Remove Cellular dependency in netsocket #13809

Uh oh!

Conversation

jeromecoutant commented Oct 23, 2020

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

Uh oh!

ciarmcom commented Oct 23, 2020

Uh oh!

pan- commented Oct 23, 2020

Uh oh!

0xc0170 left a comment • 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 Oct 26, 2020

Uh oh!

jeromecoutant commented Nov 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pan- commented Nov 4, 2020

Uh oh!

jeromecoutant commented Nov 4, 2020

Uh oh!

pan- commented Nov 4, 2020

Uh oh!

jeromecoutant commented Nov 4, 2020

Uh oh!

jeromecoutant commented Nov 9, 2020

Uh oh!

0xc0170 left a comment

Choose a reason for hiding this comment

Uh oh!

0xc0170 Nov 9, 2020

Choose a reason for hiding this comment

Uh oh!

jeromecoutant Nov 9, 2020 • 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!

jeromecoutant commented Nov 13, 2020

Uh oh!

jeromecoutant commented Nov 23, 2020

Uh oh!

0xc0170 commented Nov 23, 2020

Uh oh!

jeromecoutant commented Dec 3, 2020

Uh oh!

0xc0170 commented Dec 9, 2020

Uh oh!

mbed-ci commented Dec 9, 2020

Jenkins CI Test : ❌ FAILED

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

Uh oh!

0xc0170 commented Dec 10, 2020

Uh oh!

jeromecoutant commented Dec 17, 2020

Uh oh!

mbed-ci commented Dec 17, 2020

Jenkins CI Test : ✔️ SUCCESS

Build Number: 3 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

Uh oh!

Uh oh!

0xc0170 left a comment •

edited

Loading

jeromecoutant commented Nov 2, 2020 •

edited

Loading

jeromecoutant Nov 9, 2020 •

edited

Loading