Updated ODIN drivers to v2.5.0 RC1 #6913

asifrizwanubx · 2018-05-15T14:22:59Z

Description

This release contains the following new features.

802.11n functionality
802.11r functionality
New WiFi EMAC for UBLOX_EVK_ODIN_W2.

This release contains the following fixes.

NIL

The release was built and tested with the feature-emac branch

Test Results

armcc_mbed_test_logs.txt
gcc_mbed_test_logs.txt
iar_mbed_test_logs.txt

Pull request type

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

asifrizwanubx · 2018-05-15T15:18:23Z

Compilation errors on compiling of mbed OS tests after rebasing. @mikaleppanen

For verfication I tried it without my changes, and then for different target K64F

Build Cmd: mbed test --compile -t GCC_ARM -vv -n tests-mbedmicro-rtos-mbed-basic

Build failures:

UBLOX_EVK_ODIN_W2::GCC_ARM::TESTS-MBEDMICRO-RTOS-MBED-BASIC
Building project basic (UBLOX_EVK_ODIN_W2, GCC_ARM)

Output: arm-none-eabi-g++: error: CreateProcess: No such file or directory

Buil Cmd: mbed test --compile -t GCC_ARM -vv -n tests-mbedmicro-rtos-mbed-basic -m K64F

Build failures:

K64F::GCC_ARM::TESTS-MBEDMICRO-RTOS-MBED-BASIC
Building project basic (K64F, GCC_ARM)
Output: arm-none-eabi-g++: error: CreateProcess: No such file or directory

Following are the logs before rebasing:

armcc_mbed_test_logs.txt
gcc_mbed_test_logs.txt
iar_mbed_test_logs.txt

SeppoTakalo · 2018-05-15T16:15:35Z

Build failure is same as reported here #6861

asifrizwanubx · 2018-05-16T08:11:54Z

@SeppoTakalo

Is there any fix/solution for it? I have just reinstalled the GCC tool chain (latest) but it stills error (even on clean feature-emac).

arm-none-eabi-g++ (GNU Tools for Arm Embedded Processors 7-2017-q4-major) 7.2.1 20170904 (release) [ARM/embedded-7-branch revision 255204]
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Update: it's due to path length of project/build folder. (Fixed)

kjbracey · 2018-05-16T09:25:31Z

features/FEATURE_LWIP/lwip-interface/LWIPInterface.cpp

@@ -587,6 +587,11 @@ nsapi_error_t LWIP::Interface::bringdown()
 #if LWIP_IPV6
    mbed_lwip_clear_ipv6_addresses(&netif);
 #endif
+#if LWIP_IPV4


Why was this needed? My recollection was that lwip did zero this itself, and forgot to for IPv6, but maybe that's not true.

@andreaslarssonublox could you please respond to this?

I think it was not done for IPv4 and discovered for some case like re-connecting with static IP.

Okay, I've got no problem with this going in - don't see how it can hurt.

But let's try to avoid too many miscellaneous tweaks going in - we've got a deadline (Friday) and a misbehaving CI system.

kjbracey · 2018-05-16T09:26:42Z

features/FEATURE_LWIP/lwip-interface/lwip-sys/lwip_random.c

 #include "randLIB.h"

 void lwip_seed_random(void)
 {
+#if defined(DEVICE_TRNG)


What's going on here? LWIP is not using rand(), it's using randLIB, so this defeats lwIP's seeding.

If your driver needs rand() seeded, that's your issue, not anything to do with lwIP. And this won't achieve anything if you're not using lwIP.

If you really need to do this, it should be in your driver. However, conceptually I like to think rand() is for applications, so applications should be free to manually seed it to something non-random, without being bothered by drivers using it or seeding it in the background.

I would suggest you also use randLIB as your RNG source - that doesn't function as a a manually-seedable PRNG, so you can't interfere with other users. (And randLIB already uses the TRNG and potentially other sources as seeds).

There are two cases in this file either FEATURE_COMMON_PAL is defined or not. randLIB is only used when FEATURE_COMMON_PAL is defined if you look at the whole file. Shouldn't the seed behavior be the same for both?
We don't use rand in the driver but lwIP uses it for port assignment and depending on if FEATURE_COMMON_PAL is defined or not the behavior(seed or no seed) differs I guess?

We can remove the seeding it if it's more suited somewhere else(i.e. the application).

Look again - there aren't two cases in this file any more; FEATURE_COMMON_PAL was eliminated a few weeks back, so randLIB is always in use and the 'srand' isn't achieving anything. Presumably this is the result of a rebase.

Anyway, there's no time do be doing any more general non-EMAC-related lwIP fixes before 5.9 release - only things that are blocking actual EMAC integration like #6926

Ok, thanks. That explains it. We'll remove the change. Can you fix it @asifrizwanubx ?

kjbracey · 2018-05-16T09:27:06Z

...t/emac-drivers/TARGET_STM_EMAC/TARGET_STM32F4/TARGET_UBLOX_EVK_ODIN_W2/wifi_emac/.mbedignore

@@ -0,0 +1 @@
+


This file presumably shouldn't be being added?

kjbracey · 2018-05-16T09:31:47Z

...t/emac-drivers/TARGET_STM_EMAC/TARGET_STM32F4/TARGET_UBLOX_EVK_ODIN_W2/wifi_emac/wifi_emac.h

+#include "cb_wlan_target_data.h"
+#include "cb_wlan.h"
+
+class WIFI_EMAC : public EMAC {


I think this should be named a bit more specifically than WIFI_EMAC. Could easily collide. OdinWiFiEMAC?

ok. yes this seems fine.

@andreaslarssonublox what do you think?

kjbracey · 2018-05-16T09:33:05Z

...2F4/TARGET_STM32F439xI/TARGET_MODULE_UBLOX_ODIN_W2/sdk/ublox-odin-w2-drivers/cb_wlan_types.h

+typedef struct cbWLAN_HTCapabilities_st {
+    cbWLAN_RateMask rates;
+    cb_uint16 info;
+}cbWLAN_HTCapabilities;


Formatting (I fear feature-emac may be checked for this on merge).

I see "+42 astyle warnings from the CI".

kjbracey · 2018-05-16T09:33:28Z

...ET_STM32F439xI/TARGET_MODULE_UBLOX_ODIN_W2/sdk/ublox-odin-w2-drivers/cb_wlan_driver_config.h

+typedef enum {
+    cbWLAN_ONE_ANTENNA = 1,
+    cbWLAN_TWO_ANTENNAS
+}cbWLAN_NUMBER_OF_ANTENNAS;


kjbracey · 2018-05-16T09:33:35Z

...ET_STM32F439xI/TARGET_MODULE_UBLOX_ODIN_W2/sdk/ublox-odin-w2-drivers/cb_wlan_driver_config.h

+typedef enum {
+    cbWLAN_PRIMARY_ANTENNA_ONE = 1,
+    cbWLAN_PRIMARY_ANTENNA_TWO
+}cbWLAN_PRIMARY_ANTENNA;


kjbracey · 2018-05-16T09:35:12Z

...emac-drivers/TARGET_STM_EMAC/TARGET_STM32F4/TARGET_UBLOX_EVK_ODIN_W2/wifi_emac/wifi_emac.cpp

+
+void WIFI_EMAC::add_multicast_group(const uint8_t *address)
+{
+    // TODO anything to do here for WiFi?


Don't need to do anything as long as your interface is always returning all multicasts.

Agree, so the TODOs can be removed.

kjbracey · 2018-05-16T09:39:49Z

TESTS/network/emac/emac_TestNetworkStack.cpp

-
-    m_interface = &EmacTestNetworkStack::Interface::get_instance();
+#ifdef TARGET_UBLOX_EVK_ODIN_W2
+	emac_if_init();


There should be no need to modify the test for this. If you need to internally power up earlier than the "power up" you are free to do that internally.

Only thing you might need to do is make sure you issue "Link up" from inside the actual EMAC::power_up if you're already up.

BTW, be aware of the fix #6296 - that avoids a problem with issuing link up from inside power-up with lwip.

Could please elaborate it more? I havn't find anything in
#6296

In case of lwip emac_if_init called in a add_ethernet_interface but in test cases its quite different. odin interface need this emac initialization before the bring_up call for wifi connection messages.

this is a call stack with lwip

LWIP::Interface::emac_if_init(netif = 0x20018734) C/C++ netif_add(netif = 0x20018734, ipaddr = 0x0, netmask = 0x0, gw = 0x0, state = 0x200186d8, init = 0x80039fd <LWIP::Interface::emac_if_init(netif*)>, input = 0x8007975 <tcpip_input>) C/C++ LWIP::add_ethernet_interface(this = 0x20001848 <_ZZN4LWIP12get_instanceEvE4lwip>, emac = @0x20002018: {_vptr.EMAC = 0x8098460 <_ZTV9WIFI_EMAC+8>}, default_if = true, interface_out = 0x20011e78) C/C++ OdinWiFiInterface::handle_wlan_status_started(this = 0x20011e68, start = 0x200126c0) C/C++ OdinWiFiInterface::handle_in_msg(this = 0x20011e68) C/C++ wlan_callb_s::odin_thread_fcn(wifi = 0x20011e68) C/C++ mbed::Callback<void ()>::function_context<void (*)(OdinWiFiInterface*), OdinWiFiInterface>::operator()() const(this = 0x200120e8) C/C++ mbed::Callback<void ()>::function_call<mbed::Callback<void ()>::function_context<void (*)(OdinWiFiInterface*), OdinWiFiInterface> >(void const*)(p = 0x200120e8) C/C++ mbed::Callback<void ()>::call() const(this = 0x200120e8) C/C++ mbed::Callback<void ()>::operator()() const(this = 0x200120e8) C/C++ rtos::Thread::_thunk(thread_ptr = 0x200120e4) C/C++

And it will be only satisfied by this change

WIFI_EMAC::set_memory_manager(this = 0x20002874 <_ZZN9WIFI_EMAC12get_instanceEvE4emac>, mem_mngr = @0x20001114: {_vptr.EMACMemoryManager = 0x80887d4 <_ZTV21EmacTestMemoryManager+8>}) C/C++ emac_if_init() C/C++ EmacTestNetworkStack::add_ethernet_interface(this = 0x20001160 <_ZZN20EmacTestNetworkStack12get_instanceEvE10test_stack>, emac = @0x20002874: {_vptr.EMAC = 0x808c8f0 <_ZTV9WIFI_EMAC+8>}, default_if = true, interface_out = 0x200134f0) C/C++ OdinWiFiInterface::handle_wlan_status_started(this = 0x200134e0, start = 0x20013858) C/C++ OdinWiFiInterface::handle_in_msg(this = 0x200134e0) C/C++ wlan_callb_s::odin_thread_fcn(wifi = 0x200134e0) C/C++ mbed::Callback<void ()>::function_context<void (*)(OdinWiFiInterface*), OdinWiFiInterface>::operator()() const(this = 0x20013760) C/C++ mbed::Callback<void ()>::function_call<mbed::Callback<void ()>::function_context<void (*)(OdinWiFiInterface*), OdinWiFiInterface> >(void const*)(p = 0x20013760) C/C++ mbed::Callback<void ()>::call() const(this = 0x20013760) C/C++ mbed::Callback<void ()>::operator()() const(this = 0x20013760) C/C++ rtos::Thread::_thunk(thread_ptr = 0x2001375c) C/C++

You can internally power yourself up whenever you need to. You could have your NetworkInterface::connect do whatever initialisation you need yourself. See for example #6904, which performs its first wifi_connect powerup before calling either add_network_interface or bringup.

Your actual EMAC::powerup can be basically null, from your hardware's point of view, if you are already bringing the interface up for the "control" paths. The only issue is that if you're already up, powerup marks the point at which you should be (re)-sending the "link up" call back, as the stack will not be expecting it before then.

And #6926 (sorry, not 6296 - my typo there) corrects the logic in lwIP so it doesn't slip up if the "link up" is called from inside powerup.

In LWIP and tests, emac_if_init sets the emac memory manager "emac->set_memory_manager(*memory_manager);" and ODIN interface allocate messages by this memory manager for connection setup.

You can internally power yourself up whenever you need to.

The actual problem is the allocation of those messages by using *memory_manager and its only possible by calling emac_if_init(); before the bring_up call which is .

nsapi_error_t EmacTestNetworkStack::Interface::bringup(bool dhcp, const char *ip, const char *netmask, const char *gw, const nsapi_ip_stack_t stack, bool blocking) { if (!emac_if_init()) { TEST_ASSERT_MESSAGE(0, "emac initialization failed!"); } return NSAPI_ERROR_OK; }

@andreaslarssonublox

Got it.

How about if we guarantee that the memory manager is attached in the "add_ethernet_interface"? Would that suffice?

Not sure what do you mean by that. If it will set this

void WIFI_EMAC::set_memory_manager(EMACMemoryManager &mem_mngr) { memory_manager = &mem_mngr; }

it is sufficent.

I made a pull request to add the set_memory_manager() call to test stack Ethernet interface add function:

#6937

Will that fix the problem?

Yes, it worked.

Great. If you can give us an updated PR without the test modification (so it that relies on #6937 being present), and addresses the other comments, I can kick off combined CI to get the first pass integrated.

kjbracey · 2018-05-17T13:02:02Z

...emac-drivers/TARGET_STM_EMAC/TARGET_STM32F4/TARGET_UBLOX_EVK_ODIN_W2/wifi_emac/wifi_emac.cpp

+}
+
+// Weak so a module can override
+MBED_WEAK EMAC &EMAC::get_default_instance()


You should not be defining yourself as the default EMAC instance. That's only useful for Ethernet-type interfaces that can be run without configuration - it lets EthernetInterface pick you up.

(And this is causing double-definition compile errors versus the definition in stm32xx_emac.cpp)

Corrected EMAC tests to not to refer to "get_default_instance()" in this pull request: #6941
Update test environment with that when removing the "get_default_instance()"

cmonr · 2018-05-17T15:30:14Z

@asifrizwanubx I corrected the PR description so that only one item is selected.

Revert "in ODIN emac initialization required before connection"

asifrizwanubx · 2018-05-18T09:38:53Z

I will push one more important change. it is in build process

0xc0170 · 2018-05-18T09:39:39Z

I will push one more important change. it is in build process

Let us know once this is ready, would like to run CI

asifrizwanubx · 2018-05-18T09:40:18Z

Yes, I will let you know.

kjbracey · 2018-05-18T10:04:53Z

Can we expect it within about an hour or so? I need to decide whether it's worth kicking off a CI cycle now for other pending patches, or we should delay to get this in the same run.

asifrizwanubx · 2018-05-18T10:17:25Z

It will take some time, it is in build process and most probably it will take less than an hour.

asifrizwanubx · 2018-05-18T12:33:03Z

it's done.

kjbracey · 2018-05-18T12:57:15Z

We'll do CI for this and merge via #6936

kjbracey · 2018-05-21T09:25:07Z

Merged via #6936

kjbracey · 2018-05-21T11:21:38Z

This has been merged, but it's non-functional - we'd only been locally testing the branch tip, but the merge result with feature-emac doesn't work, due to NetworkInterface changes.

Please can you create a new PR recompiled against current feature-emac ASAP.

(This sort of compatibility problem could be mostly avoided if your outermost mbed OS glue layer was in source form.)

asifrizwanubx · 2018-05-21T11:26:44Z

Could you elaborate it more? did we miss something during compilation.

asifrizwanubx · 2018-05-21T11:31:15Z

Please can you create a new PR recompiled against current feature-emac ASAP.

Started. is there any change required in ODIN driver, EMAC?

kjbracey · 2018-05-21T11:31:37Z

Your topic branch and hence presumably the binary was based on d95acbc - a slightly old version of feature-emac.

There were some changes to NetworkInterface in the last few days - PR #6751 in particular.

We should have spotted this - testing the merge result of feature-emac with your topic branch, but we're currently in somewhat manual mode.

I don't think any code changes are required - just recompilation.

asifrizwanubx · 2018-05-21T13:25:43Z

#6963 a new PR on latest feature-emac (based on 76639b)

Asif Rizwan added 2 commits May 15, 2018 18:22

EMAC adaption added, updated ODIN drivers to v2.5.0 RC1

d3d3faa

in ODIN emac initialization required before connection

8dcf2cf

asifrizwanubx changed the title ~~Ublox odin driver os 5 v2.5.0 rc1~~ Updated ODIN drivers to v2.5.0 RC1 May 15, 2018

0xc0170 requested a review from kjbracey May 15, 2018 14:41

0xc0170 added the do not merge label May 15, 2018

kjbracey reviewed May 16, 2018

View reviewed changes

kjbracey reviewed May 17, 2018

View reviewed changes

cmonr added the needs: review label May 17, 2018

WIFI_EMAC class renamed to OdinWiFiEMAC, Formatting

19d84a5

Revert "in ODIN emac initialization required before connection"

kjbracey approved these changes May 18, 2018

View reviewed changes

mikaleppanen approved these changes May 18, 2018

View reviewed changes

WiFi EMAC class name reflected in WiFi drivers binaries

a907278

kjbracey mentioned this pull request May 18, 2018

Feature-emac combined merge #6936

Merged

kjbracey approved these changes May 18, 2018

View reviewed changes

kjbracey removed the needs: review label May 18, 2018

kjbracey closed this May 21, 2018

kjbracey removed the do not merge label May 21, 2018

asifrizwanubx mentioned this pull request Sep 25, 2018

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

Closed

Updated ODIN drivers to v2.5.0 RC1 #6913

Updated ODIN drivers to v2.5.0 RC1 #6913

Uh oh!

Conversation

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

Uh oh!

Description

This release contains the following new features.

This release contains the following fixes.

Test Results

Pull request type

Uh oh!

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

Uh oh!

Uh oh!

SeppoTakalo commented May 15, 2018

Uh oh!

asifrizwanubx commented May 16, 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!

Choose a reason for hiding this comment

Uh oh!

kjbracey May 16, 2018 • 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!

andreaslarssonublox May 16, 2018 • 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!

Choose a reason for hiding this comment

Uh oh!

andreaslarssonublox May 16, 2018 • 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!

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!

kjbracey May 16, 2018 • 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!

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!

kjbracey May 16, 2018 • 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!

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!

kjbracey May 17, 2018 • 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!

Choose a reason for hiding this comment

Uh oh!

asifrizwanubx commented May 15, 2018 •

edited

Loading

asifrizwanubx commented May 15, 2018 •

edited

Loading

asifrizwanubx commented May 16, 2018 •

edited

Loading

kjbracey May 16, 2018 •

edited

Loading

andreaslarssonublox May 16, 2018 •

edited

Loading

andreaslarssonublox May 16, 2018 •

edited

Loading

kjbracey May 16, 2018 •

edited

Loading

kjbracey May 16, 2018 •

edited

Loading

kjbracey May 17, 2018 •

edited

Loading