add-rtl8195am-feature-emac #6904

M-ichae-l · 2018-05-15T11:13:16Z

Description

rtl8195am feature emac implementation.
coding style fix.
Updated emac greentea tests #6851.

Pull request type

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

rtl8195am feature emac implementation. coding style fix. Updated emac greentea tests ARMmbed#6851.

kjbracey · 2018-05-15T11:16:39Z

Looks like you've got a local copy of #6851 included - please rebase and tidy up.

M-ichae-l · 2018-05-15T11:19:02Z

@kjbracey-arm
ok. noted.

kjbracey

We're in the process of merging feature-emac now, so we'll need fixes as soon as possible if this driver is going to make it to 5.9.0.

kjbracey · 2018-05-15T13:33:54Z

tools/test_configs/RealtekInterface.json

@@ -9,7 +9,7 @@
        },
        "connect-statement" : {
            "help" : "Disabled until EMAC updated",
-            "value" : null
+            "value" : "((RTWInterface *)net)->connect(WIFI_SSID, WIFI_PASSWORD, WIFI_SECURITY, WIFI_CHANNEL)"


Hadn't you needed an alternate version of this (net->wiFiInterface()->connect)?

Yes. It should be net->wiFiInterface()->connect

mikaleppanen

Added some review comments

mikaleppanen · 2018-05-15T14:24:17Z

targets/TARGET_Realtek/TARGET_AMEBA/RTWInterface.cpp

+
+#define LWIP 0x11991199
+#if MBED_CONF_NSAPI_DEFAULT_STACK == LWIP
+    rtw_emac.wlan_emac_link_change(true);


rtw_emac.wlan_emac_link_change() calls can be removed from here and link status call can be made on emac power up.

@mikaleppanen
the add_ethernet_interface will run emac_if_init to set the callback and then power up. Logic here is correct.
However, the netif->flags will be set by the following code. It is still cause the "connection error" from bringup().

The netif->flags setting before power up was a bug in the lwip. I made a pull request to fix that: #6926.

You can remove the callback from there and take my pull request into your test build. It should work now correctly and trigger the link up from power up.

Noted. Thank you.

Actually it seems that for second connect after disconnect, the callback is needed. It could be unconditional before the interface check e.g. like this:

rtw_emac.wlan_emac_link_change(true); if (!rtw_interface) { nsapi_error_t err = rtw_obn_stack.add_ethernet_interface(rtw_emac, true, &rtw_interface); if (err != NSAPI_ERROR_OK) { rtw_interface = NULL; return err; } }

Thanks. Will add rtw_emac.wlan_emac_link_change(true); after wifi_connect .

mikaleppanen · 2018-05-15T14:24:33Z

targets/TARGET_Realtek/TARGET_AMEBA/RTWInterface.cpp

+#define TEST 0x33254234
+#if MBED_CONF_NSAPI_DEFAULT_STACK == TEST
+    if (rtw_if_bringup == NSAPI_ERROR_OK) {
+        rtw_emac.wlan_emac_link_change(true);


Also this can be removed.

mikaleppanen · 2018-05-15T14:25:40Z

targets/TARGET_Realtek/TARGET_AMEBA/rtw_emac.cpp

    free(sg_list);
    return ret;
 }

-static bool wlan_power_up(emac_interface_t *emac)
+bool RTW_EMAC::power_up()
 {
    wifi_on(RTW_MODE_STA);
    wait_ms(1000);


Link up can be made here. e.g.
if (emac_link_state_cb) {
emac_link_state_cb(true);
}

emac_link_state_cb is the callback set by the stack, so it can be called here.

Confirmed to use wlan_emac_link_change(true);
wlan_emac_link_change(true); and

if (emac_link_state_cb) { emac_link_state_cb(true); }

actually are same. Thank you.

mikaleppanen · 2018-05-15T14:27:07Z

targets/TARGET_Realtek/TARGET_AMEBA/rtw_emac.h

+    void *emac_link_state_data;
+    emac_link_input_cb_t emac_link_input_cb; 
+    emac_link_state_change_cb_t emac_link_state_cb; 
+    LWIPMemoryManager *memory_manager; 


This should be EMACMemoryManager

Thank you. Confirmed, should use EMACMemoryManager.

fix the conflicts with ARMmbed#6851.

Expecting the ARMmbed#6926 Corrected lwip netif flags to be set before power up

kjbracey · 2018-05-16T11:26:58Z

tools/test_configs/RealtekInterface.json

@@ -9,7 +9,7 @@
        },
        "connect-statement" : {
            "help" : "Disabled until EMAC updated",
-            "value" : "((RTWInterface *)net)->connect(WIFI_SSID, WIFI_PASSWORD, WIFI_SECURITY, WIFI_CHANNEL)"
+            "value" : "(net->wifiInterface()->connect(WIFI_SSID, WIFI_PASSWORD, WIFI_SECURITY, WIFI_CHANNEL)"


Unless I'm mistaken, this has an excess '(' at the front.

Could you put back the original help from master too:

"help": "Must use 'net' variable name, replace WIFI_SSID, WIFI_PASSWORD, WIFI_SECURITY, WIFI_CHANNEL with your WiFi settings",

kjbracey · 2018-05-16T11:28:29Z

targets/TARGET_Realtek/TARGET_AMEBA/rtw_emac.cpp

 {
    return RTW_EMAC_MTU_SIZE;
 }

-static void wlan_get_ifname(emac_interface_t *emac, char *name, uint8_t size)
+uint32_t RTW_EMAC::get_align_preference() const
+{


This needs to return something. 1 is fine.

Thanks. Confirmed to add return true; for RTW_EMAC::get_align_preference() .

kjbracey · 2018-05-16T11:33:58Z

targets/TARGET_Realtek/TARGET_AMEBA/RTWInterface.cpp


+#include "LWIPStack.h"


You shouldn't be including LWIPStack.h

Thanks. Confirmed to delete LWIPStack.h .

kjbracey · 2018-05-16T11:36:00Z

targets/TARGET_Realtek/TARGET_AMEBA/RTWInterface.cpp

+        uint8_t pscan_config = PSCAN_ENABLE;
+        wifi_set_pscan_chan(&_channel, &pscan_config, 1);
+    }
+
    ret = wifi_connect(_ssid, sec, _pass, strlen(_ssid), strlen(_pass), 0, (void *)NULL);
    if (ret != RTW_SUCCESS) {
        printf("failed: %d\r\n", ret);
        return NSAPI_ERROR_NO_CONNECTION;
    }


As Mika mentioned, you will still need a "link up" callback here after the "wifi_connect", to ensure the upcall occurs if this is the second connect, and the stack is already bound to you. That's the direct mirror of the "link down" callback from "disconnect".

The callback in "power_up" then handles the case of the first connect, when the stack wasn't attached to hear this link-up callback.

Thanks. Will add rtw_emac.wlan_emac_link_change(true); after wifi_connect .

add link call back after wifi_connect add return for get_align_preference delete LWIPStack.h from RTWInterface.cpp coding style fix

kjbracey · 2018-05-17T07:48:35Z

tools/test_configs/RealtekInterface.json

-            "help" : "Disabled until EMAC updated",
-            "value" : null
+            "help" : "Must use 'net' variable name, replace WIFI_SSID, WIFI_PASSWORD, WIFI_SECURITY, WIFI_CHANNEL with your WiFi settings",
+            "value" : "(net->wifiInterface()->connect(WIFI_SSID, WIFI_PASSWORD, WIFI_SECURITY, WIFI_CHANNEL)"


This still has a stray opening '( at the front. I can edit this myself - going to need to rebase and tidy this commit.

kjbracey

Approved, but I'll rebase to tidy up and remove the opening '('.

It's passing tests here.

cmonr · 2018-05-17T15:34:05Z

/morph build

mbed-ci · 2018-05-17T16:43:40Z

Build : FAILURE

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

cmonr · 2018-05-17T16:55:25Z

@M-ichae-l Looks like all three compilers failed with a missing header against the REALTEK_RTL8195AM

[DEBUG] Output: #include "Rtw_emac.h"

M-ichae-l · 2018-05-18T02:42:00Z

@cmonr the actual files is all in lowercase. Need to change all the #include "Rtw_emac.h" and #include "Rtw_emac.cpp" become #include "rtw_emac.h" and #include "rtw_emac.cpp" .

Change all the#include "Rtw_emac.h" and #include "Rtw_emac.cpp" become #include "rtw_emac.h" and #include "rtw_emac.cpp" coding style fix

cmonr

👍

cmonr · 2018-05-18T02:47:42Z

/morph build

mbed-ci · 2018-05-18T03:16:27Z

Build : SUCCESS

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

Triggering tests

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

kjbracey · 2018-05-18T06:56:02Z

I had been trying to accelerate CI by combining the PRs here #6936 - the case fix is already made on that PR. I've stuck the "do not merge" label on the ones I've combined, so they don't need to be dealt with.

Unfortunately the CI has mostly been down within my working hours, so this hasn't really been working out.

mbed-ci · 2018-05-18T07:14:38Z

Exporter Build : SUCCESS

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

mbed-ci · 2018-05-18T09:54:18Z

Test : SUCCESS

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

* feature-emac: (61 commits) recompiled driver against NetworkInterface changes on latest feature-emac WiFi EMAC class name reflected in WiFi drivers binaries WIFI_EMAC class renamed to OdinWiFiEMAC, Formatting Revert "in ODIN emac initialization required before connection" LPC546XX: Correct Ethernet length calculations LPC546XX: Correct Ethernet MAC address write Removed EMAC get default instance from EMAC tests Add memory configuration for LPC546XX LPC546XX: Add ENET support in ODIN emac initialization required before connection EMAC adaption added, updated ODIN drivers to v2.5.0 RC1 Updated ublox odin greentea test configuration file connect statement add-rtl8195am-feature-emac (#6904) Corrected lwip netif flags to be set before power up Add NetworkInterface::get_default_instance() Build mbed-os-example-lora only for lora targets Added memory manager set to add_ethernet_interface() of test stack Changed mutexes, delete and DNS call in callback set Corrected more defects Corrected defects - Changed call_in/call methods of the stack to callback provided by the stack - Specified what are limitations for operations that are made in callback - Added pure virtual class DNS that defines DNS operations - Added cancel operation and unique ID to DNS request used in cancel - Added DNS configuration options to netsocket/mbed_lib.json for retries, response wait time and cache size - Changed host name to use dynamic memory in DNS query list and cache, set maximum length for the name to 255 bytes. - Added mutex to asynchronous DNS - Reworked retries: there is now total retry count and a server specific count - Ignores invalid incoming UDP socket messages (DNS header is not valid), and retries DNS query - Reworked DNS module asynchronous operation functions - Corrected other review issues (nothrow new, missing free, missing mutex unlock etc.) Added non-blocking DNS functionality to network interface ...

rtl8195am feature emac implementation.

add-rtl8195am-feature-emac

1eaac1a

rtl8195am feature emac implementation. coding style fix. Updated emac greentea tests ARMmbed#6851.

kjbracey requested review from kjbracey and mikaleppanen May 15, 2018 11:13

kjbracey added needs: review release-version: 5.9.0-rc1 labels May 15, 2018

kjbracey reviewed May 15, 2018

View reviewed changes

mikaleppanen reviewed May 15, 2018

View reviewed changes

M-ichae-l and others added 3 commits May 16, 2018 11:06

rebase for feacture_emac

4f529ad

fix the conflicts with ARMmbed#6851.

Merge branch 'feature-emac' into Add-rtl8195am-feature-emac

390d238

updats add rtl8915am emac

cb8b348

Expecting the ARMmbed#6926 Corrected lwip netif flags to be set before power up

kjbracey reviewed May 16, 2018

View reviewed changes

kjbracey mentioned this pull request May 16, 2018

Updated ODIN drivers to v2.5.0 RC1 #6913

Closed

update add rtl8915am emac

3364fa8

add link call back after wifi_connect add return for get_align_preference delete LWIPStack.h from RTWInterface.cpp coding style fix

kjbracey reviewed May 17, 2018

View reviewed changes

kjbracey approved these changes May 17, 2018

View reviewed changes

kjbracey mentioned this pull request May 17, 2018

Feature-emac combined merge #6936

Merged

mikaleppanen approved these changes May 17, 2018

View reviewed changes

kjbracey added do not merge and removed needs: review labels May 17, 2018

cmonr added the needs: CI label May 17, 2018

cmonr added needs: work and removed needs: CI labels May 17, 2018

update add rtl8915am emac

0641ff7

Change all the#include "Rtw_emac.h" and #include "Rtw_emac.cpp" become #include "rtw_emac.h" and #include "rtw_emac.cpp" coding style fix

cmonr approved these changes May 18, 2018

View reviewed changes

kjbracey added needs: CI and removed do not merge needs: work labels May 18, 2018

kjbracey merged commit 75f17d4 into ARMmbed:feature-emac May 18, 2018

kjbracey removed needs: CI release-version: 5.9.0-rc1 labels May 21, 2018

kjbracey pushed a commit that referenced this pull request May 22, 2018

add-rtl8195am-feature-emac (#6904)

ded8781

rtl8195am feature emac implementation.

kjbracey pushed a commit that referenced this pull request May 23, 2018

add-rtl8195am-feature-emac (#6904)

162a8c0

rtl8195am feature emac implementation.

M-ichae-l deleted the Add-rtl8195am-feature-emac branch May 30, 2018 10:48

SeppoTakalo mentioned this pull request Sep 20, 2018

[greentea] RTL8195AM netsocket test failures #8124

Closed

add-rtl8195am-feature-emac #6904

add-rtl8195am-feature-emac #6904

Uh oh!

Conversation

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

Uh oh!

Description

Pull request type

Uh oh!

kjbracey commented May 15, 2018

Uh oh!

M-ichae-l commented May 15, 2018

Uh oh!

kjbracey left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

M-ichae-l 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!

mikaleppanen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

M-ichae-l 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!

M-ichae-l 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

M-ichae-l 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!

M-ichae-l 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!

M-ichae-l 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!

M-ichae-l 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!

M-ichae-l 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

M-ichae-l commented May 15, 2018 •

edited

Loading

M-ichae-l May 16, 2018 •

edited

Loading

M-ichae-l May 16, 2018 •

edited

Loading

M-ichae-l May 17, 2018 •

edited

Loading

M-ichae-l May 16, 2018 •

edited

Loading

M-ichae-l May 16, 2018 •

edited

Loading

M-ichae-l May 17, 2018 •

edited

Loading

M-ichae-l May 17, 2018 •

edited

Loading

M-ichae-l May 17, 2018 •

edited

Loading