Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is there a race in eventmon? #1737

Closed
eyaleb opened this issue Jan 18, 2017 · 7 comments
Closed

Is there a race in eventmon? #1737

eyaleb opened this issue Jan 18, 2017 · 7 comments

Comments

@eyaleb
Copy link

eyaleb commented Jan 18, 2017

This is not a simple missed trigger bug but a design (SDK or nodemcu?) issue.

Expected behaviour

Registering an event should inform of the event happening.

Actual behaviour

The event does not always trigger.

Test code

If I do this early (to stop dhcp)

wifi.sta.setip({ip="...",netmask="...",gateway="..."})

then (at times much) later I attend to the wifi

local eventmon = wifi.eventmon
eventmon.register(eventmon.STA_CONNECTED, function(T)
    print("status="..wifi.sta.status()
end)

I usually see

status=5

However, if the registration is too late, after the connection is established, then no trigger is generated and a wdt reset takes place.

I do not see a simple way to ensure I always get notified. I now do this:

wifi.sta.setip({ip="...",netmask="...",gateway="..."})
...
local handled = false
eventmon.register(eventmon.STA_CONNECTED, function(T)
    if not handled then
        handled = true
        print("status="..wifi.sta.status())
        whatever...
    end
end)
if not handled and 5 == wifi.sta.status() then
    handled = true
    print("handling missed event")
    eventmon.unregister(eventmon.STA_CONNECTED)
    whatever...
end

but is setting handled properly serialised?

NodeMCU version

Latest dev

Hardware

Any

@djphoenix
Copy link
Contributor

Lua is event-based and have only one "thread". So lua callbacks may not ever be called simultaneously.

@eyaleb
Copy link
Author

eyaleb commented Jan 18, 2017

Thanks @djphoenix I now re-read the lua doc and can see how my code is safe, and does not even need the protection of handled.

However, the 5 == ...status section is still required (always after the registration?). It would be more reliable if the CONNECTED (and GOT_IP) could fire when the status is already satisfied at the time of registration. This will allow registering the event without worrying about the timing.

Should our doco (and examples) clarify this? Or is my understanding still lacking?

@dnc40085
Copy link
Contributor

dnc40085 commented Jan 19, 2017

I do not see a simple way to ensure I always get notified.

@eyaleb as long as the event callback is registered before the event happens, it should always fire the callback. A simple way to ensure this is to register event callbacks at the beginning of init.lua.

The event handling for wifi.eventmon is as follows:

  • The SDK calls event handler callback (in c)
  • check if user has registered an event callback (in Lua). if not, return and discard event.
  • if the user has registered an event callback (in Lua), add the event to the end of the event queue
  • Post a task to process the event queue at a later time
  • When the task executes, arrange the relevant event data in to a Lua table and return it to the user's event callback
  • remove event from queue. if event queue is not empty, post the task again.

@eyaleb
Copy link
Author

eyaleb commented Jan 19, 2017

@dnc40085 I think I now understand this process, thanks. Related though: if I unregister the event, will any corresponding queued task be removed? I expect so.

My main point remains: registering a CONNECT event does not guarantee I will know that a connection was made. I must check the status too. In short, eventmon by itself is not enough, there is a timing issue (maybe 'race' in the title is inaccurate).

@dnc40085
Copy link
Contributor

dnc40085 commented Jan 19, 2017

if I unregister the event, will any corresponding queued task be removed? I expect so.

Unfortunately, it seems I forgot to add a test for this when I implemented the event handler. thank you for pointing this out.

registering a CONNECT event does not guarantee I will know that a connection was made. I must check the status too.

I believe the eventmon.STA_CONNECTED event is indicating that the station has successfully completed the network authentication and the eventmon.STA_GOT_IP event indicates that the station now has an address and is ready to communicate with the network.

In further testing, I have found that the eventmon.STA_GOT_IP event never happens if wifi.sta.setip() is called within the eventmon.STA_CONNECTED event callback, otherwise the STA_GOT_IP event occurs every time. so you should be able to use the STA_GOT_IP event to reliably determine when you are connected to the access point and have an IP address and is network ready.

here is my test code (access point is already saved in flash)
init.lua:

do 
 wifi.sta.setip({ip = "10.0.0.105", netmask = "255.255.255.0", gateway = "10.0.0.1"}) 
 
 wifi.eventmon.register(wifi.eventmon.STA_GOT_IP, function(T) 
 print("\n\tSTA - GOT IP\n\tREADY!")
 end)

 wifi.eventmon.register(wifi.eventmon.STA_CONNECTED, function(T) 
 print("\n\tSTA - CONNECTED\n")
 end)
end

@eyaleb
Copy link
Author

eyaleb commented Jan 19, 2017

@dnc40085 You are correct, however for this issue it does not matter if I use STA_CONNECTED or STA_GOT_IP. The point is that whichever event you register, you cannot trust it to fire unless you register before the event actually happened, a point in time that is not known to your app. It is a crapshoot.

BTW, I tested, and the status is already 5 (wifi.sta.STA_GOTIP) when I enter the STA_CONNECTED event. Not that it matters for my app.

@marcelstoer
Copy link
Member

Can be considered resolved I guess.
With an event driven system you always need to register the callbacks before the function that triggers those events is invoked.

It's just a bit unfortunate that we've got both wifi.sta.eventMonReg() and wifi.eventmon.register(). Our documentation states:

Note: The functions wifi.sta.eventMon___() and wifi.eventmon.___() are completely seperate and can be used independently of one another.

but w/o giving recommendations as for when to use what that isn't really helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants