Extend and rework WiFi interface #2627

bulislaw · 2016-09-06T09:43:55Z

General refactoring of the API and two new methods added:

get_rssi() - measures radio signal strenght
get_state() - which returns current state (not connected,
connecting, connected, error)

I changed how the get_rssi function works, comparing to what Lauri proposed. It's more in line in how other IOT SDKs work and also it's simpler. It only fetches current RSSI, if application wants to average it out over multiple measurements it can always wrap it in a loop, but I think it's probably good enough for most people.

lauri-piikivi · 2016-09-06T10:19:23Z

customer wanted the method to scan_ssid to find available networks. I would make it simple, return a list of found networks, which can be empty if error happened. this method should be added at same time as this rss and state methods

bulislaw · 2016-09-06T10:20:14Z

OK, will add it now then.

bulislaw · 2016-09-06T13:01:28Z

scan added. I would say we are still missing some basic functionality like using static IP (not DHCP), but that would require changing underlying NetworkInterface (we really should have it for Ethernet as well) which may not fit the current time scales. Something to remember about.

jupe · 2016-09-06T17:03:12Z

features/net/network-socket/WiFiInterface.h

+     * @return       Number of entries in @a, or if @a count was 0 number of available networks, negative on error
+     *               see @a nsapi_error
+     */
+    virtual int scan(wifi_ap_t *res, unsigned count) = 0;


this might be problematic api for application. Only way is to estimate how many networks there might be available, but still you might get just partial list of ssid's. It would be more usefull and also memory friendly if you could get one by one result, or if this api allow to skip N results...

I am +1 for this solution. It is simple and should work for most use cases.

Correct me if I am wrong, but alternatives would require implicit memory allocation for devices that return all results in a single query (I believe the ESP8266 for one).

This is not yet implemented to esp8266-driver, or is it ? I just look at quickly AT command which give you list of access points, and it depend how implement will be done that is it possible to get one by one those results easily or not. At least ESP8266 give you whole list of access points so you have to reserve memory for all of them dynamically before returning back.. but it probably would be much faster if you could get results one by one as soon as you get result from wifi chip.. there is cons and prons in both ways..

I'm moving ESP implementation together with this API https://github.com/bulislaw/esp8266-driver/tree/feature_wifi
There's no way of getting it one by one really, the async api call the callback between paring next entry.

async api looks good alternative solution which do the trick "I wanted" 👍 . I think this sync API is good enough for now.

lauri-piikivi · 2016-09-07T06:21:53Z

there is the problem of blocking call for scan and connect. If those are changed to be event based, then the scan could return SSIDs (APs) one by one as it finds them. The status method would be event values.

Connect and scan can take 30secs and more

Customer did raise the concern that this is blocking API

bulislaw · 2016-09-07T08:22:30Z

We can't really do a 'continue' index because each scan is different, some new networks can appear, but also the order is different. We could do event based scan, or even better add extra event based function, but depending on module that may be impossible to implement properly as they may return whole list of detected networks anyway.

I agree that having both sync/async API is the best solution. I'll add the event based methods.

geky · 2016-09-08T19:17:00Z

features/net/network-socket/WiFiInterface.h

+     *
+     * @return Connection strength in dBm (negative value), or 0 if measurement impossible
+     */
+    virtual int16_t get_rssi() = 0;


Why int16_t? Just curious

Mostly because I wasn't sure int8_t is large enough.

i believe that 8bit is enough for rssi..

bogdanm · 2016-09-09T14:10:16Z

features/net/network-socket/WiFiInterface.h

+     * @param  cb    Function to be called for every discovered network
+     * @param  data  A user handle that will be passed to @a cb along with the AP data
+     */
+    virtual void scan(wifi_ap_scan_cb_t cb, void *data = NULL) = 0;


Nitpick (sorry, it's Friday): I'd call this something like scan_async (and maybe even scan_sync for the blocking version). Even though the documentation is very clear, calling both functions sync and having one be synchronous and the other one asynchronous can lead to some confusion for people reading the code. Makes even more sense since you already did this with connect_async.

Agreed, makes sense.

bogdanm · 2016-09-09T14:11:04Z

What would be the best way to test this PR?

bulislaw · 2016-09-09T14:13:20Z

There's ESP8266 implementation for the new API: https://github.com/bulislaw/esp8266-driver/tree/feature_wifi

0xc0170 · 2016-09-13T13:16:36Z

This is for a wifi feature branch where we can progress with it. I would go with the new return types as @bulislaw proposing, and clean the rest meanwhile (that can happen separately), the feature wifi branch is just for wifi.

Any other leftovers for this to be merged to the feature branch?

bulislaw · 2016-09-13T13:18:08Z

I think that's it, if we find something else needs tweaking we can fix it in subsequent commits.

sg- · 2016-09-14T04:14:37Z

I'd suggest not modifying the return types for now. This can be done later given it needs to apply to the whole API, not just Wi-Fi
async needs work. attach member is the precident for this with an enum of what the event is to be fired on. More docs on this coming soon.

Looks good to merge and keep taking patches pending CI results of course 👍

sg- · 2016-09-14T04:14:45Z

/morph test

mbed-bot · 2016-09-14T05:22:28Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 856

Build failed!

sg- · 2016-09-14T05:38:33Z

Failure not related but if possible I'd suggest rebasing this branch against master so development CI at least compiles. There is a include path problem when compiling on linux due to case sensitivity for the NCS platform.

a929bbb

General refactoring of the API and new methods added: * get_rssi() - measures radio signal strenght * get_state() - returns current state (not connected, connecting, connected, error) * scan() - scans for available networks sync and async versions * connect_async() - connect to a network without blocking

bulislaw · 2016-09-14T12:00:11Z

Rebased.

0xc0170 · 2016-09-14T12:07:43Z

/morph test

mbed-bot · 2016-09-14T14:41:50Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 861

All builds and test passed!

bridadan · 2016-09-14T15:28:33Z

Just for complete clarity, the build failure was due to the fact that PR #2678 was not present on the branch this PR was being merged into. When @bulislaw rebased he brought in that change, hence the build passed.

bulislaw force-pushed the feature_wifi branch from de2c44d to 7b4d948 Compare September 6, 2016 12:56

bulislaw force-pushed the feature_wifi branch 2 times, most recently from 09e0f10 to 984424e Compare September 6, 2016 13:49

jupe reviewed Sep 6, 2016
View reviewed changes

bulislaw force-pushed the feature_wifi branch 4 times, most recently from 9f7de44 to 64b9da7 Compare September 8, 2016 09:32

geky reviewed Sep 8, 2016
View reviewed changes

bulislaw force-pushed the feature_wifi branch from 64b9da7 to 21a2f0f Compare September 9, 2016 09:51

bogdanm reviewed Sep 9, 2016
View reviewed changes

bogdanm added needs: review needs: CI labels Sep 9, 2016

bulislaw force-pushed the feature_wifi branch from 21a2f0f to 0a3d0cd Compare September 9, 2016 14:17

geky mentioned this pull request Sep 12, 2016

nsapi - Add optional API for static configuration of network interfaces #2664

Merged

bulislaw force-pushed the feature_wifi branch from 0a3d0cd to d19c0c3 Compare September 13, 2016 12:27

sg- removed the needs: review label Sep 14, 2016

bulislaw force-pushed the feature_wifi branch from d19c0c3 to f9f1f44 Compare September 14, 2016 11:57

0xc0170 removed the needs: CI label Sep 15, 2016

0xc0170 merged commit 4fde794 into ARMmbed:feature_wifi Sep 15, 2016

geky mentioned this pull request Oct 19, 2016

nsapi - Add standardized return types for size and errors #3075

Merged

3 tasks

Extend and rework WiFi interface #2627

Extend and rework WiFi interface #2627

Uh oh!

Conversation

bulislaw commented Sep 6, 2016

Uh oh!

lauri-piikivi commented Sep 6, 2016

Uh oh!

bulislaw commented Sep 6, 2016

Uh oh!

bulislaw commented Sep 6, 2016

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!

lauri-piikivi commented Sep 7, 2016

Uh oh!

bulislaw commented Sep 7, 2016

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!

bogdanm commented Sep 9, 2016

Uh oh!

bulislaw commented Sep 9, 2016

Uh oh!

0xc0170 commented Sep 13, 2016

Uh oh!

bulislaw commented Sep 13, 2016

Uh oh!

sg- commented Sep 14, 2016

Uh oh!

sg- commented Sep 14, 2016

Uh oh!

mbed-bot commented Sep 14, 2016

Output

Uh oh!

sg- commented Sep 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bulislaw commented Sep 14, 2016

Uh oh!

0xc0170 commented Sep 14, 2016

Uh oh!

mbed-bot commented Sep 14, 2016

Output

Uh oh!

bridadan commented Sep 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

sg- commented Sep 14, 2016 •

edited

Loading

bridadan commented Sep 14, 2016 •

edited

Loading