Update ODIN drivers to v3.0.0 RC1 #7579

asifrizwanubx · 2018-07-23T10:46:55Z

Description

This release contains the following new features.

Access Point functionality
Bridge functionality
Robustness

This release contains the following fixes.

Instantiating OdinWiFiInterface seems to be changing Thread behavior with I2C Slave #6122

Known limitations

Only possible to use either Wi-Fi AP or Wi-Fi Station

Test Results

The following tests have been performed

mbed-os test for GCC_ARM, ARM and IAR
mbed-os WiFi tests (target for Robustness)

iar_network_wifi_logs.txt
arm_mbed_logs.txt
arm_network_wifi_logs.txt
gcc_mbed_logs.txt
gcc_network_wifi_logs.txt
iar_mbed_logs.txt

Other

To enable the bridge add the below macro and increase the lwIP pbuf pool size in mbed_app.json, for example:
"macros": ["MBED_EMAC_LWIP_L2_BRIDGE=1"],
"lwip-pbuf-pool-size":
{ "help": "Number of pbuf pool buffers", "value": "80" }
To enable the AP add the below macro in mbed_app.json, for example:
"macros": ["DEVICE_WIFI_AP=1"],

Pull request type

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

mlubinsky · 2018-07-23T23:33:07Z

How to get this patch #7579
into my project, which is a fork of
https://github.com/armmbed/ref-wem-firmware
?
I am using mbed-cli. I need to test it ASAP.

0xc0170 · 2018-07-24T08:29:44Z

@mlubinsky Fetch this via remote. One of the approaches could be

git checkout -b u-blox-ublox_odin_driver_os_5_v3.0.0_rc1 master
git pull https://github.com/u-blox/mbed-os.git ublox_odin_driver_os_5_v3.0.0_rc1

0xc0170 · 2018-07-25T11:39:58Z

I can see now why this was marked as feature. First two commits : one is fixing tests, another one is adding new feature. I believe this 2 shall be sent separately (odin update driver would depend on them).

Can you split this PR please

asifrizwanubx · 2018-07-26T06:09:31Z

revert the changes related to wifi tests, will be push in a seperate PR. @0xc0170

asifrizwanubx · 2018-07-26T06:11:21Z

@AmmadRehmat could you please create a new PR for fix of WiFi tests?

SeppoTakalo · 2018-07-26T08:35:47Z

@asifrizwanubx
Consider enabling all network tests.

Building project tcp (UBLOX_EVK_ODIN_W2, GCC_ARM)
Scan: GCC_ARM
Scan: tcp
Compile [  8.3%]: main.cpp
[Error] main.cpp@19,2: #error [NOT_SUPPORTED] No network configuration found for this target.
[Error] main.cpp@59,25: 'MBED_CONF_APP_CONNECT_STATEMENT' was not declared in this scope
[Error] main.cpp@72,36: 'MBED_CONF_APP_ECHO_SERVER_ADDR' was not declared in this scope
[Error] main.cpp@73,23: 'MBED_CONF_APP_ECHO_SERVER_PORT' was not declared in this scope
[Error] main.cpp@82,36: 'MBED_CONF_APP_ECHO_SERVER_ADDR' was not declared in this scope
Building project stats_heap (UBLOX_EVK_ODIN_W2, GCC_ARM)

In order to run TCP and UDP tests you need to provide test configs in mbed_app.json or as a test config.json. Look for samples in mbed-os/tools/test-configs

0xc0170 · 2018-07-26T08:49:22Z

revert the changes related to wifi tests, will be push in a seperate PR. @0xc0170

Thanks, is adding WiFiSoftAPInterface.h is part of this PR as it relates to the driver update ?

SeppoTakalo · 2018-07-26T08:52:13Z

I don't see where this flag DEVICE_WIFI_AP is defined or documented.

Also, I don't much like this #ifdeffing that changes the class totally.

Why would you not have separate class that implements the AP functions on top of UbloxOdinInterface?

So instead of

#ifdef DEVICE_WIFI_AP
+class OdinWiFiInterface : public WiFiInterface, public WiFiSoftAPInterface, public EMACInterface
#else
class OdinWiFiInterface : public WiFiInterface, public EMACInterface
#endif

You would do:

class OdinWiFiInterface : public WiFiInterface, public EMACInterface {
...
};
class OdinSoftAPInterface: public OdinWifiInterface, public WifiSoftAPInterface {
 // Soft AP functions here
}

Would allow you to get rid of #ifdef and still allows linker to drop unused AP functions. (Unless there is some runtime checks in your .cpp code that are disabled by #ifndef DEVICE_WIFI_AP)

asifrizwanubx · 2018-07-27T07:26:30Z

@0xc0170

is adding WiFiSoftAPInterface.h is part of this PR as it relates to the driver update ?

yes it is part of this PR without it driver with AP functionality didn't work.

asifrizwanubx · 2018-07-27T07:32:30Z

@SeppoTakalo

I don't see where this flag DEVICE_WIFI_AP is defined or documented.

We have added the description of DEVICE_WIFI_AP in PR and by this macro we will restrict the application use of station and AP at a time. Otherwise it will not work.

We will deliver soon a public example for AP with proper documentation.

Could you please point out where will I add description of DEVICE_WIFI_AP?

screamerbg · 2018-08-01T10:46:11Z

@SeppoTakalo do you have further requests/suggestions?

SeppoTakalo · 2018-08-01T11:14:21Z

I need @kjbracey-arm and @mikaleppanen to review this, but Kevin is still on vacation.

This PR adds WifiSoftAPInterface as part of public Mbed OS API so it needs more eyes and thoughts. We need to decide how to document and support this after it becomes part of Mbed OS.

AmmadRehmatCUS · 2018-08-07T06:34:22Z

@SeppoTakalo
Attaching test reports for tcp and udp tests.
tcp.log
udp.log

Consider enabling all network tests.

Building project tcp (UBLOX_EVK_ODIN_W2, GCC_ARM)
Scan: GCC_ARM
Scan: tcp
Compile [ 8.3%]: main.cpp
[Error] main.cpp@19,2: #error [NOT_SUPPORTED] No network configuration found for this target.
[Error] main.cpp@59,25: 'MBED_CONF_APP_CONNECT_STATEMENT' was not declared in this scope
[Error] main.cpp@72,36: 'MBED_CONF_APP_ECHO_SERVER_ADDR' was not declared in this scope
[Error] main.cpp@73,23: 'MBED_CONF_APP_ECHO_SERVER_PORT' was not declared in this scope
[Error] main.cpp@82,36: 'MBED_CONF_APP_ECHO_SERVER_ADDR' was not declared in this scope
Building project stats_heap (UBLOX_EVK_ODIN_W2, GCC_ARM)

In order to run TCP and UDP tests you need to provide test configs in mbed_app.json or as a test config.json. Look for samples in mbed-os/tools/test-configs

SeppoTakalo · 2018-08-07T07:51:32Z

@AmmadRehmat Thanks.

Please note that you can enable full set of TCP+UDP tests by enabling macro MBED_EXTENDED_TESTS

https://github.com/ARMmbed/mbed-os/blob/master/TESTS/netsocket/tcp/main.cpp#L137
https://github.com/ARMmbed/mbed-os/blob/master/TESTS/netsocket/udp/main.cpp#L94

AmmadRehmatCUS · 2018-08-07T10:06:27Z

Attaching extended testing reports.
udp_extended_log.txt
tcp_extended_log.txt

Please note that you can enable full set of TCP+UDP tests by enabling macro MBED_EXTENDED_TESTS

https://github.com/ARMmbed/mbed-os/blob/master/TESTS/netsocket/tcp/main.cpp#L137
https://github.com/ARMmbed/mbed-os/blob/master/TESTS/netsocket/udp/main.cpp#L94

SeppoTakalo · 2018-08-07T10:31:30Z

Looks good.

0xc0170

Cosmetic changes only

0xc0170 · 2018-08-07T14:07:09Z

features/FEATURE_LWIP/lwip-interface/emac_lwip_l2_bridge.cpp

+
+#include "emac_lwip_l2_bridge.h"
+#include "string.h"
+#include "mbed.h"


Can this be replaced by header files that are required here? Rather than one header file mbed.h

0xc0170 · 2018-08-07T14:07:18Z

features/FEATURE_LWIP/lwip-interface/emac_lwip_l2_bridge.cpp

@@ -0,0 +1,413 @@
+/* mbed Microcontroller Library
+ * Copyright (c) 2016 ARM Limited


2018 year for new files

0xc0170 · 2018-08-07T14:08:12Z

features/lwipstack/LWIPMemoryManager.cpp

+void LWIPMemoryManager::ref(emac_mem_buf_t *buf)
+{
+    pbuf_ref(static_cast<struct pbuf *>(buf));
+}


Wasnt there a new line at the end of this line? Github shows there is none now, this results in warning in some toolchains

Since we are working on cosmetic changes, I have noticed that in release 5.9.0, feature_lwip folder contains files which have been moved to lwipstack folder. In 5.9.3 we only have 2 files in folder feature_lwip, emac_lwip_l2_bridge.h and emac_lwip_l2_bridge.c. Should these files be moved to lwipstack folder as well?

mikaleppanen · 2018-08-08T11:19:19Z

features/FEATURE_LWIP/lwip-interface/emac_lwip_l2_bridge.cpp

+    err_t   res;
+    bool    free_msg = true;
+
+    _mem.ref(buf);


Reference counting has not been added to general EMAC memory manager interface (EMACMemoryManager.h), but since it is needed only for this lwip specific brigde, I'm not against implementing it only to lwip memory manager for now.

mikaleppanen · 2018-08-08T11:28:56Z

features/netsocket/WiFiSoftAPInterface.h

+     *  @param ssid      Name of the network to connect to
+     *  @param pass      Security passphrase to connect to the network
+     *  @param security  Type of encryption for connection (Default: NSAPI_SECURITY_NONE)
+     *  @param channel   Channel on which the connection is to be made, or 0 for any (Default: 0)


Is there support for "0" setting? Seems to fail in is_valid_AP_channel() with channel set to zero.

The description is a little bit misleading, we will fix the description. Channel zero is not allowed but error handling for non supporting channels is available. Will fix the description soon.

There are also similar descriptions about channel 0 in OdinWiFiInterface.h and in set_ap_channel() function call. They should be updated too.

cmonr · 2018-08-09T01:30:55Z

@0xc0170 @mikaleppanen Mind re-reviewing?

asifrizwanubx · 2018-08-09T08:11:35Z

@mikaleppanen @0xc0170 please review it again. I made the changes according to your feedback.

mikaleppanen · 2018-08-10T09:46:52Z

...RGET_STM32F439xI/TARGET_MODULE_UBLOX_ODIN_W2/sdk/ublox-odin-w2-drivers/OdinWiFiInterface.cpp

+            nsapi_error_t error_code = _stack.add_ethernet_interface(_emac, true, &_interface);
+            if (error_code != NSAPI_ERROR_OK) {
+                _interface = NULL;
+            }


Like in EMACInterface::connect(), there needs to be call here to "_interface->attach(_connection_status_cb);" in else branch to add the callback to interface. Otherwise callbacks attached between the construct and connect() do not work.

is #6562 issue due to this?

I think that #7310 could be caused by this.

AmmadRehmatCUS · 2018-08-31T09:58:20Z

@kjbracey-arm commit squashed and cleaned.

0xc0170 · 2018-09-01T08:33:42Z

needs: review again. Needs @kjbracey-arm approval

0xc0170 · 2018-09-02T18:24:02Z

/morph build

mbed-ci · 2018-09-02T20:16:31Z

Build : SUCCESS

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

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

cmonr · 2018-09-02T20:17:06Z

@kjbracey-arm @mikaleppanen #7579 (comment)

Re-requesting that this PR be re-reviewewed. Comments look to be addressed.

mbed-ci · 2018-09-02T22:34:18Z

Exporter Build : SUCCESS

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

mbed-ci · 2018-09-02T23:59:07Z

Test : FAILURE

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

cmonr · 2018-09-03T01:48:45Z

Interesting. Test failures don't look related to PR.
/morph test

mbed-ci · 2018-09-03T03:41:03Z

Test : SUCCESS

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

mbed-ci · 2018-09-03T07:46:13Z

Exporter Build : FAILURE

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

flipflop22 · 2018-10-01T08:42:51Z

I have been trying to use this AP feature. It does not work well.
I can see the Network is there, however I can't connect a station (another ODIN-W2) to the AP

asifrizwanubx · 2018-10-01T09:19:53Z

@flipflop22

Could you please share the error or issue?

Or try this example.
https://os.mbed.com/teams/ublox/code/mbed-os-example-odinw2-wifi-ap/

If still you face some issue please report it.

0xc0170 added the needs: review label Jul 23, 2018

0xc0170 requested a review from a team July 23, 2018 11:10

mlubinsky mentioned this pull request Jul 23, 2018

Instantiating OdinWiFiInterface seems to be changing Thread behavior with I2C Slave #6122

Closed

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

asifrizwanubx mentioned this pull request Jul 31, 2018

TCPsocket and UDPsocket Speed #7204

Closed

cmonr added the release-version: 5.10.0-rc1 label Aug 6, 2018

0xc0170 previously requested changes Aug 7, 2018

View reviewed changes

mikaleppanen suggested changes Aug 8, 2018

View reviewed changes

cmonr added needs: review and removed needs: work labels Aug 9, 2018

mikaleppanen suggested changes Aug 10, 2018

View reviewed changes

AmmadRehmatCUS force-pushed the ublox_odin_driver_os_5_v3.0.0_rc1 branch from ce8ac8f to b934632 Compare August 31, 2018 06:40

cmonr added needs: CI and removed needs: work labels Aug 31, 2018

0xc0170 added needs: review and removed needs: CI labels Sep 1, 2018

kjbracey approved these changes Sep 3, 2018

View reviewed changes

mikaleppanen approved these changes Sep 3, 2018

View reviewed changes

0xc0170 added ready for merge and removed needs: review labels Sep 3, 2018

0xc0170 merged commit 44925d8 into ARMmbed:master Sep 3, 2018

0xc0170 removed the ready for merge label Sep 3, 2018

0xc0170 changed the title ~~Updated ODIN drivers to v3.0.0 RC1~~ Update ODIN drivers to v3.0.0 RC1 Sep 3, 2018

This was referenced Sep 4, 2018

UBLOX_EVK_ODIN_W2 does not pass Greentea test-case WIFI_SET_CREDENTIAL #5645

Closed

UBLOX_EVK_ODIN_W2 does not pass Greentea testcase WIFI-CONNECT-PARAMS-CHANNEL and WIFI-CONNECT-PARAMS-CHANNEL-FAIL #5582

Closed

screamerbg mentioned this pull request Sep 10, 2018

Network interface functionality #6562

Closed

This was referenced Sep 24, 2018

Ublox EVK ODIN W2 does not pass WIFI-CONNECT-PARAMS-CHANNEL-FAIL test case #8227

Closed

Ublox EVK ODIN W2 does not pass WIFI-GET-RSSI test case #8228

Closed

UBLOX_EVK_ODIN_W2 target failed on mbed HAL lp_ticker/sleep/rtc greentea test #8196

Closed

		@@ -0,0 +1,413 @@
		/* mbed Microcontroller Library
		* Copyright (c) 2016 ARM Limited

Update ODIN drivers to v3.0.0 RC1 #7579

Update ODIN drivers to v3.0.0 RC1 #7579

Uh oh!

Conversation

asifrizwanubx commented Jul 23, 2018

Description

This release contains the following new features.

This release contains the following fixes.

Known limitations

Test Results

Other

Pull request type

Uh oh!

mlubinsky commented Jul 23, 2018

Uh oh!

0xc0170 commented Jul 24, 2018

Uh oh!

0xc0170 commented Jul 25, 2018

Uh oh!

asifrizwanubx commented Jul 26, 2018

Uh oh!

asifrizwanubx commented Jul 26, 2018

Uh oh!

SeppoTakalo commented Jul 26, 2018

Uh oh!

0xc0170 commented Jul 26, 2018

Uh oh!

SeppoTakalo commented Jul 26, 2018

Uh oh!

asifrizwanubx commented Jul 27, 2018

Uh oh!

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

Uh oh!

Uh oh!

screamerbg commented Aug 1, 2018

Uh oh!

SeppoTakalo commented Aug 1, 2018

Uh oh!

AmmadRehmatCUS commented Aug 7, 2018

Uh oh!

SeppoTakalo commented Aug 7, 2018

Uh oh!

AmmadRehmatCUS commented Aug 7, 2018

Uh oh!

SeppoTakalo commented Aug 7, 2018

Uh oh!

0xc0170 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!

cmonr commented Aug 9, 2018

Uh oh!

asifrizwanubx commented Aug 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

asifrizwanubx commented Jul 27, 2018 •

edited

Loading

asifrizwanubx commented Aug 9, 2018 •

edited

Loading

asifrizwanubx commented Oct 1, 2018 •

edited

Loading