Component: Add qca400x wifi #11466

mmahadevan108 · 2019-09-12T03:57:00Z

Description

Add support for QCA400X Wifi module. This is used on the LPC55S69 Xpresso board. Wifi test results below:

mbedgt: test suite report:
| target              | platform_name | test suite                 | result | elapsed_time (sec) | copy_method |
|---------------------|---------------|----------------------------|--------|--------------------|-------------|
| LPC55S69_NS-GCC_ARM | LPC55S69      | mbed-os-tests-network-wifi | OK     | 97.93              | default     |
mbedgt: test suite results: 1 OK
mbedgt: test case report:
| target              | platform_name | test suite                 | test case                          | passed | failed | result | elapsed_time (sec) |
|---------------------|---------------|----------------------------|------------------------------------|--------|--------|--------|--------------------|
| LPC55S69_NS-GCC_ARM | LPC55S69      | mbed-os-tests-network-wifi | WIFI-CONNECT                       | 1      | 0      | OK     | 8.33               |
| LPC55S69_NS-GCC_ARM | LPC55S69      | mbed-os-tests-network-wifi | WIFI-CONNECT-DISCONNECT-REPEAT     | 1      | 0      | OK     | 26.37              |
| LPC55S69_NS-GCC_ARM | LPC55S69      | mbed-os-tests-network-wifi | WIFI-CONNECT-NOCREDENTIALS         | 1      | 0      | OK     | 0.06               |
| LPC55S69_NS-GCC_ARM | LPC55S69      | mbed-os-tests-network-wifi | WIFI-CONNECT-PARAMS-CHANNEL        | 1      | 0      | OK     | 0.11               |
| LPC55S69_NS-GCC_ARM | LPC55S69      | mbed-os-tests-network-wifi | WIFI-CONNECT-PARAMS-CHANNEL-FAIL   | 1      | 0      | OK     | 0.11               |
| LPC55S69_NS-GCC_ARM | LPC55S69      | mbed-os-tests-network-wifi | WIFI-CONNECT-PARAMS-NULL           | 1      | 0      | OK     | 0.06               |
| LPC55S69_NS-GCC_ARM | LPC55S69      | mbed-os-tests-network-wifi | WIFI-CONNECT-PARAMS-VALID-SECURE   | 1      | 0      | OK     | 1.85               |
| LPC55S69_NS-GCC_ARM | LPC55S69      | mbed-os-tests-network-wifi | WIFI-CONNECT-PARAMS-VALID-UNSECURE | 1      | 0      | OK     | 2.88               |
| LPC55S69_NS-GCC_ARM | LPC55S69      | mbed-os-tests-network-wifi | WIFI-CONNECT-SECURE                | 1      | 0      | OK     | 1.93               |
| LPC55S69_NS-GCC_ARM | LPC55S69      | mbed-os-tests-network-wifi | WIFI-CONNECT-SECURE-FAIL           | 1      | 0      | OK     | 18.37              |
| LPC55S69_NS-GCC_ARM | LPC55S69      | mbed-os-tests-network-wifi | WIFI-CONSTRUCTOR                   | 1      | 0      | OK     | 0.04               |
| LPC55S69_NS-GCC_ARM | LPC55S69      | mbed-os-tests-network-wifi | WIFI-GET-RSSI                      | 1      | 0      | OK     | 1.45               |
| LPC55S69_NS-GCC_ARM | LPC55S69      | mbed-os-tests-network-wifi | WIFI-SCAN                          | 1      | 0      | OK     | 1.4                |
| LPC55S69_NS-GCC_ARM | LPC55S69      | mbed-os-tests-network-wifi | WIFI-SCAN-NULL                     | 1      | 0      | OK     | 1.92               |
| LPC55S69_NS-GCC_ARM | LPC55S69      | mbed-os-tests-network-wifi | WIFI-SET-CHANNEL                   | 1      | 0      | OK     | 0.08               |
| LPC55S69_NS-GCC_ARM | LPC55S69      | mbed-os-tests-network-wifi | WIFI-SET-CREDENTIAL                | 1      | 0      | OK     | 0.05               |
mbedgt: test case results: 16 OK

Pull request type

[ ] Fix
[ ] Refactor
[X] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

Release Notes

mmahadevan108 · 2019-09-12T03:58:51Z

cc @0xc0170 @maclobdell @linlingao

ciarmcom · 2019-09-12T05:00:27Z

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

0xc0170 · 2019-09-12T05:16:18Z

Please review travis failure

0xc0170 · 2019-09-12T05:16:51Z

[X] Functionality change

I believe this is a target update (adding already supported functionality).

VeijoPesonen · 2019-09-12T06:08:40Z

This is nitpicking but would you please update the PR's description in a way that tests results are marked as code to make those a bit more readable.

VeijoPesonen · 2019-09-12T06:10:22Z

components/wifi/COMPONENT_QCA400X_WIFI/QCA_WiFiInterface.h

+#ifndef QCA_WIFIINTERFACE_H
+#define QCA_WIFIINTERFACE_H
+
+#include "mbed.h"


mbed.h should be used only by applications and not inside Mbed OS itself.

Updated and removed this include

VeijoPesonen · 2019-09-12T06:18:59Z

components/wifi/COMPONENT_QCA400X_WIFI/QCA_WiFiInterface.cpp

+#include "platform/mbed_debug.h"
+#include "platform/mbed_wait_api.h"
+#include <stdio.h>
+#define TRACE_GROUP  "QCAI" // ESP8266 Interface


The comment should be fixed.

Thank you for spotting this.

VeijoPesonen · 2019-09-12T06:20:47Z

components/wifi/COMPONENT_QCA400X_WIFI/QCA_WiFiInterface.cpp

+    if (level == NSAPI_SOCKET && socket->proto == NSAPI_TCP) {
+        switch (optname) {
+            case NSAPI_KEEPALIVE: {
+                if (socket->connected) { // ESP8266 limitation, keepalive needs to be given before connecting


The comment should be fixed.

Thank you for spotting this.

michalpasztamobica · 2019-09-12T06:42:49Z

I wonder if the path should not be shortened to: components/wifi/qca400x ? The COMPONENT and WIFI parts can be inferred from the path anyway.
If we want to follow the existing naming convention there name should be lower case and could be followed by -driver although I do not insist on this, as I personally think it is not necessary.

EDIT: if this is meant to be a component, then I need to modify this comment, just suggesting that we drop the _WIFI suffix.

michalpasztamobica · 2019-09-12T07:52:07Z

components/wifi/COMPONENT_QCA400X_WIFI/QCA_WiFiInterface.cpp

+
+using namespace mbed;
+
+QCA_WiFiInterface::QCA_WiFiInterface():


Is this a generic interface for all QCA devices? Are there any other QCA devices? Can QCA serve any other purpose than Wifi? Please, consider those questions and if possible change the class name to something as accurate as possible like QCA400XInterface.

Thank you. Sure, I will make this change

Thanks for adding the 400X part. Do we need the Wifi part for any reason? I also wonder about the underscore. I read the Coding style guidelines and all it says is that every word should start with a capital letter. Underscores are not explicitly forbidden, but I don't think this particular underscore can be justified in here?
I am personally a fan of QCA400XInterface - the Wifi part just seems redundant (unless there is a module with the same name, that is not a Wifi module?)
@0xc0170 , @VeijoPesonen, any toughts on this?

We do have OdinWiFiInterface

@VeijoPesonen , Odin does not clearly indicate a wifi chip, but a board family, so I imagine it might have more drivers and related Odin*Interface classes. QCA400x is a WiFi-only chip, so it will probably have no other Interface than a wifi one :).
I think Wifi is redundant, but if noone else sees this is as a problem, I don't want to hold back the review. Let's just clarify the mbed_lib.json name, that I mentioned in the other comment.

I will change to drop the Wifi from the name

michalpasztamobica · 2019-09-12T08:46:48Z

components/wifi/COMPONENT_QCA400X_WIFI/QCA_WiFiInterface.cpp

+    return ret;
+}
+
+nsapi_error_t QCA_WiFiInterface::set_blocking(bool blocking)


If the non-blocking mode ways the same way it does for ESP, then I suggest just returning UNSUPPORTED. @VeijoPesonen , what do you think?
Please see this PR for how we are trying to get ESP really non-blocking

If the non-blocking mode ways the same way it does for ESP, then I suggest just returning UNSUPPORTED.

Yes, I do agree.

OK, I will make the change

michalpasztamobica · 2019-09-12T08:49:17Z

components/wifi/COMPONENT_QCA400X_WIFI/mbed_lib.json

+{
+    "name": "qca400x",
+    "macros": [
+        "A_LITTLE_ENDIAN"


Do we need that A_ at the beginning? Is this just an indefinite article? If so - I suggest it gets dropped :)

This is something the wifi_qca driver looks for, I am using the driver as it.

I see, perhaps it's "A" for "Aetheros"...

michalpasztamobica · 2019-09-12T08:55:17Z

targets/targets.json

@@ -2123,7 +2123,7 @@
            "NXP_LPADC",
            "MBED_TICKLESS"
        ],
-        "components_add": ["FLASHIAP"],
+        "components_add": ["FLASHIAP", "QCA400X_WIFI"],


Is this the reason behind the folder naming convention COMPONENT_QCA400X_WIFI that I mentioned in my other comment? We want this to be an addable component? If so, the please just consider dropping the _WIFI, unless it is not obvious that QCA400X can only act as a WIFI module :-)

I can drop the WIFI, the naming convention follows what is done for CELLULAR component

I am confused... you dropped the COMPONENT_ part and left the _WIFI in... Does this still work fine?
Is there any reason to keep the _WIFI suffix? @0xc0170 can you give your recommendations on the naming or do you know who can?

COMPONENT is not needed as it will be added during build as it is part of the components_add section. I left the WIFI in the folder name as this is the convention followed by cellular.

It should be fine as it is now

mmahadevan108 · 2019-09-12T15:25:40Z

I wonder if the path should not be shortened to: components/wifi/qca400x ? The COMPONENT and WIFI parts can be inferred from the path anyway.
If we want to follow the existing naming convention there name should be lower case and could be followed by -driver although I do not insist on this, as I personally think it is not necessary.

EDIT: if this is meant to be a component, then I need to modify this comment, just suggesting that we drop the _WIFI suffix.

I followed the naming convention for cellular

mmahadevan108 · 2019-09-13T21:07:25Z

Below are the test results, I have updated the PR to address all feedback and the CI failure.
WiFi_Test.txt

0Grit · 2019-09-14T14:32:31Z

@ARMmbed/team-embeddedplanet

0xc0170 · 2019-09-18T12:37:21Z

@mmahadevan108 Please review astyle failures

mmahadevan108 · 2019-09-18T13:41:50Z

@mmahadevan108 Please review astyle failures

I will fix the files that I added. But there is a lot of failures in the Atheros driver files (everything in the folder wifi_qca) which were not created by me. Is there a way to make an exception for these?

mmahadevan108 · 2019-09-19T12:22:20Z

@mmahadevan108 Please review astyle failures

I will fix the files that I added. But there is a lot of failures in the Atheros driver files (everything in the folder wifi_qca) which were not created by me. Is there a way to make an exception for these?

@0xc0170 Is it possible to make an exception for the driver files not added by me or should I fix all style issues?

0xc0170 · 2019-09-19T13:18:56Z

@0xc0170 Is it possible to make an exception for the driver files not added by me or should I fix all style issues?

Yes, add these to astyleignore files. We should not change 3rd party files.

mmahadevan108 · 2019-09-19T14:35:27Z

@0xc0170 Is it possible to make an exception for the driver files not added by me or should I fix all style issues?

Yes, add these to astyleignore files. We should not change 3rd party files.

Is there an example I can use.

0xc0170 · 2019-10-01T13:50:52Z

@mmahadevan108 Sorry for the delay, yes. Check .astyleignore file in the root of this repo.

mmahadevan108 · 2019-10-08T15:26:25Z

Thank you, I have updated this PR

0xc0170 · 2019-10-17T13:49:10Z

@mmahadevan108 There are still multiple failures in the changed files, please review astyle job

mmahadevan108 · 2020-02-05T19:09:22Z

I have rebased this PR. I am able to pass all the TCP tests under netsocket. However I am seeing the below failure when running the TLS test

[1580929570.99][CONN][RXD] >>> Running case #8: 'TLSSOCKET_HANDSHAKE_INVALID'...
[1580929571.03][CONN][INF] found KV pair in stream: {{__testcase_start;TLSSOCKET_HANDSHAKE_INVALID}}, queued...
[DBG ][TLSW]: mbedtls_ssl_conf_ca_chain()
[INFO][TLSW]: Starting TLS handshake with (null)
[DBG ][TLSW]: mbedtls_ssl_setup()
[ERR ][TLSW]: mbedtls_ssl_handshake() failed: -0x2700 (-9984): X509 - Certificate verification failed, e.g. CRL, CA or signature check failed

Any suggestions how I can fix this?

michalpasztamobica · 2020-02-05T20:57:11Z

Hi @mmahadevan108 . First thing to try - I would suggest increasing stack and heap sizes, TLS consumes quite some memory (The certificate alone weighs over 1600B).
Does this happen for all compilers?
You may also want to try enabling TLS logging to see the exact reason for failure - might be something unrelated to networking. This thread explains how to go about this.

mergify · 2020-02-10T13:05:26Z

This PR cannot be merged due to conflicts. Please rebase to resolve them.

mmahadevan108 · 2020-02-19T03:56:03Z

I am seeing failure with the below test:

[1582065953.56][CONN][RXD] >>> Running case #14: 'UDPSOCKET_ECHOTEST_NONBLOCK_CONNECT_SEND_RECV'...
[1582065953.64][CONN][INF] found KV pair in stream: {{__testcase_start;UDPSOCKET_ECHOTEST_NONBLOCK_CONNECT_SEND_RECV}}, queued...
[INFO][GRNT]: Packets sent: 66, packets received 0, loss ratio 1.00

Calling UDPSOCKET_ECHOTEST_NONBLOCK_impl() function with a true argument passes but fails with a false argument.

Any suggestions?

mmahadevan108 · 2020-02-25T23:33:48Z

The connect function at the below line
https://github.com/ARMmbed/mbed-os/blob/master/TESTS/netsocket/udp/udpsocket_echotest.cpp#L185

Is not calling the connect inside my driver. It calls the one inside InternetDatagramSocket.cpp. Any idea why?

0xc0170 · 2020-02-27T12:08:43Z

The connect function at the below line
https://github.com/ARMmbed/mbed-os/blob/master/TESTS/netsocket/udp/udpsocket_echotest.cpp#L185

Is not calling the connect inside my driver. It calls the one inside InternetDatagramSocket.cpp. Any idea why?

@ARMmbed/mbed-os-ipcore Could you help?

kivaisan · 2020-02-27T12:37:01Z

The connect function at the below line
https://github.com/ARMmbed/mbed-os/blob/master/TESTS/netsocket/udp/udpsocket_echotest.cpp#L185

Is not calling the connect inside my driver. It calls the one inside InternetDatagramSocket.cpp. Any idea why?

@ARMmbed/mbed-os-ipcore Could you help?

Because this is using UDPSocket and it does not have "real" connect method like TCPSocket but like a "simulated connect". So calling connect for UDPSocket only stores the destination address (https://github.com/ARMmbed/mbed-os/blob/master/features/netsocket/InternetDatagramSocket.cpp#L21) and when application calls send it will call driver's socket_sendto method with the address given in connect. And recv just returns packets using socket_recvfrom but only from this address.

This UDPSOCKET_ECHOTEST_CONNECT_SEND_RECV is made to ensure UDPSocket can be used like TCPSocket. This is not visible on driver level.

mmahadevan108 · 2020-02-27T16:00:28Z

Thank you. I found an issue in my drivers recvfrom implementation. I was able to move further

Signed-off-by: Mahesh Mahadevan <[email protected]>

Malloc failures were seen when running WiFi tests Signed-off-by: Mahesh Mahadevan <[email protected]>

0xc0170 · 2020-02-27T16:59:51Z

CI restarted

mbed-ci · 2020-02-27T17:58:41Z

Test run: FAILED

Summary: 3 of 4 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-IAR

mmahadevan108 · 2020-02-28T13:06:26Z

The netsocket-dns tests are failing. However the test was able to resolve using DNS server (8.8.8.8) when running TCP, UDP tests. Is there something else I am missing in my implementation that these tests are exercising,

maclobdell · 2020-02-28T20:26:46Z

I'm trying to test the latest commits on this PR.

Specifically I'm trying to run this test:

mbed test -m LPC55S69_NS -t ARM -n tests-integration-net-single

Getting a TCPSocket API error.

Is this because of the API deprecation changes on the master branch for Mbed OS 6? Perhaps this is an issue with our integration tests not being updated to latest APIs.

@0xc0170 - we hope this PR could be merged and released with the next 5.15.x. How do you recommend testing it? Am I doing something wrong?

mmahadevan108 · 2020-02-28T22:51:20Z

The netsocket-dns tests are failing. However the test was able to resolve using DNS server (8.8.8.8) when running TCP, UDP tests. Is there something else I am missing in my implementation that these tests are exercising,

The DNS tests are failing with the error NSAPI_ERROR_TIMEOUT

0xc0170 · 2020-03-24T15:26:57Z

@mmahadevan108 Have you found a root cause for these failures?

I'll restart CI now to get the latest results (build will still fail?)

0xc0170 · 2020-03-24T15:37:26Z

jenkins-ci/build-IAR — Failure

needs to be ignored in the status filed.

mbed-ci · 2020-03-24T16:04:53Z

Test run: FAILED

Summary: 2 of 3 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM

0xc0170 · 2020-03-25T09:07:14Z

[ERROR] In file included from ./mbed-os/components/wifi/COMPONENT_QCA400X_WIFI/wifi_qca/common_src/driver/driver_diag.c:26: In file included from ./mbed-os/components/wifi/COMPONENT_QCA400X_WIFI/wifi_qca/custom_src/include/a_config.h:29: ./mbed-os/components/wifi/COMPONENT_QCA400X_WIFI/wifi_qca/port/wifi_common.h:23:10: fatal error: 'wifi_shield.h' file not found #include "wifi_shield.h" failed

0xc0170 · 2020-04-06T12:26:35Z

@mmahadevan108 Lets close this one and reopen once it's updated

ciarmcom requested review from a team September 12, 2019 05:00

ciarmcom added the needs: review label Sep 12, 2019

0xc0170 added needs: work and removed needs: review labels Sep 12, 2019

VeijoPesonen reviewed Sep 12, 2019

View reviewed changes

michalpasztamobica reviewed Sep 12, 2019

View reviewed changes

mmahadevan108 force-pushed the Add_QCA400X_Wifi branch 2 times, most recently from 4b81d8f to 7727fde Compare September 13, 2019 20:55

mmahadevan108 force-pushed the Add_QCA400X_Wifi branch from 7727fde to f292c3d Compare October 8, 2019 15:24

mmahadevan108 force-pushed the Add_QCA400X_Wifi branch from f292c3d to ba98613 Compare October 16, 2019 14:28

mmahadevan108 force-pushed the Add_QCA400X_Wifi branch from 08d36fa to 336c02d Compare February 18, 2020 21:38

mmahadevan108 added 4 commits February 27, 2020 10:58

LPC55S69: Add API to get the Chip revision

f07a9a9

Signed-off-by: Mahesh Mahadevan <[email protected]>

QCA400X: Add wifi support for this module

7f82fb2

Signed-off-by: Mahesh Mahadevan <[email protected]>

astyle: exclude wifi_qca

f59827c

Signed-off-by: Mahesh Mahadevan <[email protected]>

LPC55S69: Increase the heap size for IAR toolchain

b2c7a64

Malloc failures were seen when running WiFi tests Signed-off-by: Mahesh Mahadevan <[email protected]>

mmahadevan108 force-pushed the Add_QCA400X_Wifi branch from 336c02d to b2c7a64 Compare February 27, 2020 16:58

0xc0170 added needs: CI and removed needs: work labels Mar 24, 2020

0xc0170 added needs: work and removed needs: CI labels Mar 25, 2020

0xc0170 closed this Apr 6, 2020

mergify bot removed the needs: work label Apr 6, 2020


		using namespace mbed;

		QCA_WiFiInterface::QCA_WiFiInterface():

Component: Add qca400x wifi #11466

Component: Add qca400x wifi #11466

Uh oh!

Conversation

mmahadevan108 commented Sep 12, 2019 • edited by 0xc0170 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Reviewers

Release Notes

Uh oh!

mmahadevan108 commented Sep 12, 2019

Uh oh!

ciarmcom commented Sep 12, 2019

Uh oh!

0xc0170 commented Sep 12, 2019

Uh oh!

0xc0170 commented Sep 12, 2019

Uh oh!

VeijoPesonen commented Sep 12, 2019

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!

michalpasztamobica commented Sep 12, 2019 • 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!

michalpasztamobica Sep 16, 2019 • 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!

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!

mmahadevan108 commented Sep 12, 2019

Uh oh!

mmahadevan108 commented Sep 13, 2019

Uh oh!

0Grit commented Sep 14, 2019

Uh oh!

0xc0170 commented Sep 18, 2019

Uh oh!

mmahadevan108 commented Sep 12, 2019 •

edited by 0xc0170

Loading

michalpasztamobica commented Sep 12, 2019 •

edited

Loading

michalpasztamobica Sep 16, 2019 •

edited

Loading

kivaisan commented Feb 27, 2020 •

edited

Loading