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

Update init.lua example to use new wifi.sta.config() implementation #1953

Merged
merged 5 commits into from
May 5, 2017

Conversation

dnc40085
Copy link
Contributor

@dnc40085 dnc40085 commented May 4, 2017

Fixes #1951 .

  • This PR is for the dev branch rather than for master.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/en/*.

When I updated the function wifi.sta.config() to accept a table for station configuration, I forgot to update the init.lua example in docs/en/upload.md accordingly.

The new version of the example also makes use of the event monitor rather than a timer to catch station events.

-- Define WiFi station event callbacks
wifi_connect_event = function(T)
print("Connection to AP("..T.SSID..") established!")
print("Wating for ip...")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo. Should be Waiting.

@jmattsson
Copy link
Member

👍 in general

@dnc40085 dnc40085 force-pushed the dev_update_init.lua_example branch from 1cb7551 to 1d80d02 Compare May 4, 2017 05:03
@marcelstoer
Copy link
Member

marcelstoer commented May 5, 2017

Woah, a really elaborate template, thanks. Did you consider to drop the "Inspired by " note at the end (it has deviated quite a bit)? I'll add some notes/questions inline.

@jmattsson what does your cautious "in general" mean? I interpret this as if you were not fully satisfied with the proposal?

-- Define WiFi station event callbacks
wifi_connect_event = function(T)
print("Connection to AP("..T.SSID..") established!")
print("Waiting for ip...")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about "Waiting for IP address..."?

for key,val in pairs(wifi.eventmon.reason) do
if val == T.reason then
print("Disconnect reason:"..T.reason.."("..key..")")
reason_string=key
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you keep reason_string? I don't see it being used anywhere...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to add the disconnect reason to the final abort message, but I decided not to and I forgot to remove this variable.

--the list and returns the string corresponding to the disconnect reason.
for key,val in pairs(wifi.eventmon.reason) do
if val == T.reason then
print("Disconnect reason:"..T.reason.."("..key..")")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about "Disconnect reason: "..val.." ("..key..")"?

return
end
-- total_tries: how many times the station will attempt to connect to the AP.
local total_tries = 3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a default value I would increase this significantly (or drop it altogether) to accommodate for intermittent AP reboots.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does 30 sound good?

Copy link
Member

@marcelstoer marcelstoer May 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does but I haven't tested how this translates to time. Would it be safe to assume that APs take anywhere between 30s and 60s to boot?

Copy link
Contributor Author

@dnc40085 dnc40085 May 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My Comcast Xfinity gateway(Arris technicolor tc8305c) takes about 2.5 min to reboot and the interval between NO_AP_FOUND disconnect events is about 2.25 seconds, so I think a value of 75 should cover most cases.

@jmattsson
Copy link
Member

Oh, just the typo at the time :)
No objections from me, merge whenever you're satisfied!

@dnc40085 dnc40085 force-pushed the dev_update_init.lua_example branch from c5d9ae9 to d77ba57 Compare May 5, 2017 10:42
@marcelstoer marcelstoer added this to the 2.0.0-follow-up milestone May 5, 2017
@marcelstoer marcelstoer merged commit e491f4b into nodemcu:dev May 5, 2017
@dnc40085 dnc40085 deleted the dev_update_init.lua_example branch May 5, 2017 11:37
eiselekd pushed a commit to eiselekd/nodemcu-firmware that referenced this pull request Jan 7, 2018
…odemcu#1953)

* Update init.lua example in upload.md with new station config format
* Fixed typo in description of wifi.eventmon.register()
* Fixed typo and improved example init.lua in docs/en/upload.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants