Expose SDK event monitor #1018

dnc40085 · 2016-02-07T11:41:39Z

This PR exposes the SDK event monitor discussed in issue #870
wifi.sta.eventMonxxx code was moved to wifi_eventmon.c
changed wifi.sta.eventMonReg(): callbacks can now be unregistered by leaving function field blank.
changed wifi.sta.eventMonStop(): all callbacks can now be unregistered by using 1 instead of "unreg all"
All backwards compatibility was preserved. The event monitor documentation can be found here.

Also included is the addition of the functions wifi.ap.deauth()(click here for documentation) and wifi.sta.getrssi()(click here for documentation)

pjsg · 2016-02-08T14:18:01Z

Does the SDK event monitor callback execute at interrupt level or at base level? I would hope at base level.....

dnc40085 · 2016-02-09T01:48:05Z

@pjsg Honestly, I have no idea how to find that out and the SDK documentation doesn't provide that much information, just the function wifi_set_event_handler_cb(), a very short description and some example code.

I thought rewording the commit would replace the original, oh well...

marcelstoer · 2016-02-09T20:15:32Z

I thought rewording the commit would replace the original, oh well...

If that's your intention then you need to squash/amend your commits and then force push them (git push --force). Force pushing will erase the comments based on code lines in the previous commit, though. The comments in this conversion will stay as they're not part of the repository.

dnc40085 · 2016-02-11T01:25:25Z

I also discovered the undocumented function wifi_softap_deauth which goes perfectly with the event monitor, unauthorized stations can be deauthed when they are connecting.

I have written an implementation for the function, should I add it to this pull request?

Here's the documentation on the function if anybody is interested.

jmattsson · 2016-02-13T04:45:16Z

app/include/rom.h

@@ -131,6 +132,8 @@ int ets_printf(const char *format, ...)  __attribute__ ((format (printf, 1, 2)))

 int ets_sprintf(char *str, const char *format, ...)  __attribute__ ((format (printf, 2, 3)));

+int ets_vsprintf (char *d, const char *s, va_list ap);


This is not part of the boot ROM. It's provided by the SDK libs. This definition (and the old ets_sprintf() one above) should go into sdk-overrides/include/ets_sys.h.

Thank you for clarifying that, I wasn't sure where to put it.

jmattsson · 2016-02-13T04:52:31Z

With the new wifi.eventmon.register(), does one still have to call wifi.sta.eventMonStart() ? What about if I'm only interested in monitoring AP events?

dnc40085 · 2016-02-13T09:30:45Z

The functions wifi.eventmon.register() and wifi.sta.eventMonReg() are completely separate and can be used independently from one another and even disabled in user_config.h.

The wifi.sta.eventmon___() functions use a timer to check wifi_station_get_connect_status() every 150ms and when the returned value changes the function executes a user registered callback.
The purpose of the function wifi.sta.eventMonStart() is to arm the timer.

The function wifi.eventmon.register() registers the user's callback function so it can later be retrieved when the SDK calls the event handler wifi_handle_event_cb, which first checks for a user callback. Then, if found, it uses switch/case to determine which event was returned by the SDK then assembles the available information into a table and passes it to the user's callback function.

I was thinking maybe I should change "wifi.eventmon" to something like "wifi.eventhandler" to prevent confusion with the "wifi.sta.eventMon__" functions, any thoughts?

dnc40085 · 2016-02-20T03:09:47Z

Event monitor now copies event data into a queue and posts a task(using new tasking interface) to process the queue.

I also added a fix for issue #1065.

dnc40085 · 2016-02-21T03:08:47Z

^^^I'm guessing this failure is related to #1070^^^

TerryE · 2016-03-05T08:43:17Z

@dnc40085 I'll do a top to tail review of this this weekend and if OK then I'll merge it. Sorry for the delay.

dnc40085 · 2016-03-05T09:29:39Z

No worries, I'm in no rush.

I was wondering... locally, I was reworking wifi.sta.config to be simpler(use a stack pointer) and I found a bug where station can't connect to an open AP and I wrote a fix, should I add another commit to this or should I submit a separate PR?

Also, is LUA_USE_MODULES_WIFI_EVENTMON OK in user_modules.h or should I do away with it and just use the entries I already have in user_config.h?

TerryE · 2016-03-05T10:22:20Z

Also, is LUA_USE_MODULES_WIFI_EVENTMON OK in user_modules.h or should I do away with it and just use the entries I already have in user_config.h?

As far as the application programmer is concerned this is just a config option within wifi, so I would not have the extra LUA_USE but stick with the enable option in the config file.

marcelstoer · 2016-03-17T21:17:19Z

@dnc40085 We don't have an agreement as for #1146 but this PR is not (yet?) on that list, it may not make the cut. So, I feel it'd be helpful if you could create a dedicated PR for the wifi.NULLMODE fix. That one could then be merged rather sooner than later independently from this one. The relevant bug report #1034 has already been closed a while ago which is confusing.

TerryE · 2016-03-17T23:51:05Z

Perhaps we should apply a 90:10 test "Is this a material step forward over the current code in master/dev?". I am always worried that we contributors are all time limited and if we apply a 100% rule then we'll just end up never moving forward.

On this basis I would merge this now, but not without at least one other contributor +1.

dnc40085 · 2016-03-19T03:11:01Z

So what should I do? split it off?

TerryE · 2016-03-19T12:24:19Z

@dnc40085

I took a look at the disassembled code for the the function wifi_set_event_handler_cb and I found a call to the function ets_timer_handler_isr, so I'm inclined to say that the callback is executed at interrupt level rather than base level.

If you look gist then you will see that this is offset by 0xCC which is in ets_timer_init() so I am inclined to differ. 😄

Nope IMO leave it as it is now now. 90:10 rules.

jmattsson · 2016-03-30T04:57:12Z

This PR has been lingering for some time now even after all the updates. What are the arguments against merging this? Is it the "half-module" that Marcel brought up? If that's the only thing I'm in favour of merging now and improving later.

dnc40085 · 2016-03-30T07:09:12Z

If anybody has any questions about the PR please feel free to ask.
Suggestions for changes are also welcome.

dnc40085 · 2016-03-30T07:21:15Z

@TerryE

If you look gist then you will see that this is offset by 0xCC which is in ets_timer_init() so I am inclined to differ. 😄

You're probably right, my understanding of assembler is limited.

TerryE · 2016-03-30T09:06:08Z

@dnc40085 are you happy for us to merge this?

dnc40085 · 2016-03-30T17:36:54Z

I have no more changes to make to this PR, Aside from not being sure if I should remove the pseudo future proofing section (If Espressif adds any new events, this section allows the user to assign a default callback to execute if an event specific callback is not defined).

Should I squash the commits before the merge?

jmattsson · 2016-03-31T01:44:14Z

Yes please., might as well keep it neat and tidy!

Implement SDK event monitor Move wifi status event monitor code into seperate file (app/modules/wifi_eventmon.c) Modify wifi lua callback registration code. Add Functions wifi.ap.deauth and wifi.sta.getrssi Rework wifi event monitor to use tasking interface fix for Lua coroutine compatibility issue Made changes Suggested by TerryE Also, moved code that sets the default host name out of luaopen_wifi_init and into a separate function and added a post_task_low entry in it's place. Replaced some if test then return error lines with luaL_argcheck Add check for malloc null return in wifi.eventmon to catch out of memory errors

dnc40085 · 2016-04-01T02:09:15Z

The commits have been squashed.

jmattsson · 2016-04-01T02:15:45Z

Thanks! Sorry about the delay to get this merged.

dnc40085 · 2016-04-01T02:26:31Z

No problem. Thanks for merging this.

dnc40085 force-pushed the dev_new_event_monitor branch 6 times, most recently from 5cb0028 to 758221b Compare February 8, 2016 10:22

dnc40085 force-pushed the dev_new_event_monitor branch from 758221b to d1e25b1 Compare February 9, 2016 01:24

dnc40085 force-pushed the dev_new_event_monitor branch 2 times, most recently from cbebd05 to 2ef289a Compare February 11, 2016 00:54

dnc40085 mentioned this pull request Feb 11, 2016

wifi.setmode(wifi.NULLMODE) reports "wrong arg type" #1034

Closed

jmattsson reviewed Feb 13, 2016
View reviewed changes

dnc40085 force-pushed the dev_new_event_monitor branch from 2ef289a to 3195b43 Compare February 13, 2016 08:11

dnc40085 force-pushed the dev_new_event_monitor branch 3 times, most recently from fc67664 to 7b14053 Compare February 13, 2016 15:13

dnc40085 changed the title ~~Implement SDK event monitor~~ Expose SDK event monitor Feb 15, 2016

dnc40085 force-pushed the dev_new_event_monitor branch from 8aa7286 to c487225 Compare February 19, 2016 13:30

dnc40085 mentioned this pull request Feb 20, 2016

wifi.sta.eventMonReg previous_State is always "nil" #1065

Closed

dnc40085 force-pushed the dev_new_event_monitor branch 4 times, most recently from 20a568e to 391e4cc Compare February 21, 2016 03:03

dnc40085 force-pushed the dev_new_event_monitor branch 5 times, most recently from be0b217 to 160c5bc Compare March 5, 2016 02:46

dnc40085 force-pushed the dev_new_event_monitor branch from 160c5bc to 9630e8f Compare March 5, 2016 12:21

This was referenced Mar 7, 2016

Change a number of default values #1127

Merged

Add optional callbacks to wifi.sta.connect() and wifi.sta.disconnect() #1131

Closed

dnc40085 force-pushed the dev_new_event_monitor branch from 570beca to 8af8fb7 Compare March 17, 2016 15:55

dnc40085 force-pushed the dev_new_event_monitor branch from 8af8fb7 to 5e9ab01 Compare April 1, 2016 02:02

jmattsson merged commit 0556bf8 into nodemcu:dev Apr 1, 2016

dnc40085 mentioned this pull request Apr 7, 2016

Update modules to be Lua coroutine compatible #1167

Closed

dnc40085 deleted the dev_new_event_monitor branch August 2, 2016 09:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose SDK event monitor #1018

Expose SDK event monitor #1018

dnc40085 commented Feb 7, 2016

pjsg commented Feb 8, 2016

dnc40085 commented Feb 9, 2016

marcelstoer commented Feb 9, 2016

dnc40085 commented Feb 11, 2016

jmattsson Feb 13, 2016

dnc40085 Feb 13, 2016

jmattsson commented Feb 13, 2016

dnc40085 commented Feb 13, 2016

dnc40085 commented Feb 20, 2016

dnc40085 commented Feb 21, 2016

TerryE commented Mar 5, 2016

dnc40085 commented Mar 5, 2016

TerryE commented Mar 5, 2016

marcelstoer commented Mar 17, 2016

TerryE commented Mar 17, 2016

dnc40085 commented Mar 19, 2016

TerryE commented Mar 19, 2016

jmattsson commented Mar 30, 2016

dnc40085 commented Mar 30, 2016

dnc40085 commented Mar 30, 2016

TerryE commented Mar 30, 2016

dnc40085 commented Mar 30, 2016

jmattsson commented Mar 31, 2016

dnc40085 commented Apr 1, 2016

jmattsson commented Apr 1, 2016

dnc40085 commented Apr 1, 2016

		@@ -131,6 +132,8 @@ int ets_printf(const char *format, ...) __attribute__ ((format (printf, 1, 2)))

		int ets_sprintf(char str, const char format, ...) __attribute__ ((format (printf, 2, 3)));

		int ets_vsprintf (char d, const char s, va_list ap);

Expose SDK event monitor #1018

Expose SDK event monitor #1018

Conversation

dnc40085 commented Feb 7, 2016

pjsg commented Feb 8, 2016

dnc40085 commented Feb 9, 2016

marcelstoer commented Feb 9, 2016

dnc40085 commented Feb 11, 2016

jmattsson Feb 13, 2016

Choose a reason for hiding this comment

dnc40085 Feb 13, 2016

Choose a reason for hiding this comment

jmattsson commented Feb 13, 2016

dnc40085 commented Feb 13, 2016

dnc40085 commented Feb 20, 2016

dnc40085 commented Feb 21, 2016

TerryE commented Mar 5, 2016

dnc40085 commented Mar 5, 2016

TerryE commented Mar 5, 2016

marcelstoer commented Mar 17, 2016

TerryE commented Mar 17, 2016

dnc40085 commented Mar 19, 2016

TerryE commented Mar 19, 2016

jmattsson commented Mar 30, 2016

dnc40085 commented Mar 30, 2016

dnc40085 commented Mar 30, 2016

TerryE commented Mar 30, 2016

dnc40085 commented Mar 30, 2016

jmattsson commented Mar 31, 2016

dnc40085 commented Apr 1, 2016

jmattsson commented Apr 1, 2016

dnc40085 commented Apr 1, 2016