Wifi improvements #199
Wifi improvements #199
Conversation
enviro/__init__.py
Outdated
| @@ -153,56 +153,105 @@ def stop_activity_led(): | |||
| print(" - -- ---- -----=--==--=== hey enviro, let's go! ===--==--=----- ---- -- - ") | |||
| print("") | |||
|
|
|||
| def reconnect_wifi(ssid, password, country): | |||
| import time | |||
There was a problem hiding this comment.
I would suggest to match the 2 spaces used by the original code, because right now it's almost impossible to see what's actually different
There was a problem hiding this comment.
Indeed, that was an error. Adjusted.
|
I notice that this PR has been open for the last few months, but from the comments it doesnt look like it is pending any work after September. Are there any plans to merge this into any specific future release? I have 12 enviro indoor's deployed, and manually adding these changes to each of them is rather time consuming. So would prefer to wait for a release if there are plans for that already. |
|
@basil-dsouza I am ready for it to be reviewed for merge. I emailed Pimoroni a couple of months ago and was advised that while this project wasn't abandoned, it also wasn't under active review at the moment due to other priorities. It was briefly revisited after that with some preparatory PRs processed, but I've not seen any activity since. So while it's not impossible this will be revisited, I wouldn't say that it is likely either. |
|
please review and reject or merge, to say this project is isn't under active review isn't great, this is a paid for product, which the software has been opened up to the community to evolve which is great, but it needs to be maintained or handed over, but as this is the core software of a paid for product I'd be disappointed to not see either merge and release, or a rejection of this PR rather than leaving it open |
|
I think us users are going to have to call out pimoroni and get them to activly maintain this project or add another person that can do what they can not be bothered to do. I need the battery status that was " temporarly removed" months ago added back as my enviro weather is sitting in a box in the cupboard doing nothing as it is unusable as it stands. It works for weeks or months then boom its dead becuase the batteries have died, I am disabled and need to arrange to get someone to go and change batteries, at least if I knew they were close to discharged I could make prior arrangements .. |
|
Sorry for the radio silence here. I won't waste words on excuses. @sjefferson99 thank you for taking the time to identify, fix and communicate these WiFi issues. Are you comfortable rebasing this pull request to remove the merges? And, more so, willing to do so? That should give some clean commits I can merge and also kick the CI to produce some more test builds. I am hoping to merge this PR and produce an interim release. Following that, I need to run some tests, merge and release the new MicroPython v1.22.1 firmware ( I can't comment on any specific outstanding bugs - particularly the issue with battery level monitoring - but I am spending some time to get up to speed with this project and see if I'm able to help. |
sjefferson99
force-pushed
the
wifi-improvements
branch
from
75b4030 to
21e0a18
Compare
|
@Gadgetoid I've had a quick google and a test on a spare repo and I believe I have achieved the necessary rebasing of the merges from main. Let me know if this is what you intended and appears correct. |
|
@sjefferson99 nailed it. Thank you. For anyone logged into GitHub, a full firmware with these fixes is available from the CI - https://github.com/pimoroni/enviro/actions/runs/7545806918/artifacts/1174690874 And the filesystem only build - https://github.com/pimoroni/enviro/actions/runs/7545806918/artifacts/1174690873 You can usually find these be expanding the PR checks, clicking "Details", navigating to "Summary" and scrolling down for the artifacts. Edit: By way of a small update- I haven't forgotten about this, but the family have been struck down by covid for the last week (and now I'm joining in the fun) so I haven't had an opportunity to test it on my boards and sign off on a release. |
|
I'm still very much focussed on fixing the unholy mess Pi 5 and Bookworm OS have made of our Pi software ecosystem, which is why I've blinked and another month has flashed by on this PR. It's still on my radar, though. By way of an update- There's a bug fixed in the MicroPython/cyw43 side of things for wireless networks without a password that we'll probably want to try and include in a new release. (micropython/micropython@699477d) I'm currently over on our main repository testing a bump to MicroPython v1.22.2, which is another patch release that snuck up on me. Current Enviro releases currently track our release v1.20.4 which is based upon MicroPython v1.20 (April 2023) so it's quite behind 😬, but the recent release add their own little bit of fun to the mix- upgrading LittleFS so you can't downgrade from a newer firmware to an older one without losing your filesystem (though in Enviro's case, where Python code and firmware are almost one and the same, this might be less of a problem). I remember @ZodiusInfuser mentioning somewhere that this changeset, and I quote, "adds a new issue of thinking it's connected to a WiFi network when it hasn't", though I am not clear on the circumstances that reveal this bug. No doubt my thoroughly terrible wireless network might surface them. Some feedback from anyone who has used this build (available from CI as mentioned above) would be really helpful. Thank you! |
|
My weather station has been out of action recently, but I've just put it back up. It's running enviro firmware v1.22.1 but on the latest branch of my own fork which is now continuously running and multicore, but still uses chunks of original enviro code and I'll feed back any weird behaviour that could be related to Enviro code. I have seen wifi networks thinking they're connected when they're not, but usually when developing and ungracefully ripping the power from the board and not resetting the board. Also seen anecdotal issues getting an IP address when developing and reconnecting, then powering off very often, almost like some kind of resource exhaustion is reached where time fixes it. I've switched to hard resetting the board on every file upload in vscode now as it's the only way my other project that runs close to the wire on RAM can start up. Not sure if I've seen it since changing to that reset behaviour in development. @Gadgetoid If you need any help testing anything on Enviro or Pimoroni pico-w firmware, let me know presumably on a new issue or PR, I'll see what I can do. |
|
be nice to get an official release of this now - the WIFI stability the interim release supplies is a huge benefit, anything beyond that is a bonus or should not block a major stability upgrade and can be worked on for a later release. @sjefferson99 huge thanks for. your efforts on this, it's made a real difference to me |
|
@Gadgetoid I've found in further testing that this code was disconnecting on every check even when connected, which is due to the following line from @mrexodia original code at line number 216: of __init__.py. I believe it should be: To not disconnect when a healthy connection is in place. This is working much better on my test setup. I can make this change on my branch for this PR, but equally if that will delay this entering the main code branch, I can raise another PR when this is committed. |
|
@sjefferson99 did you make the change in this PR, it doesn't look like it from what I can see, the change you suggested looks beyond minor and fixes an incorrect 'equal to' check. This needs releasing |
|
@ikonia It's definitely worth fixing, but I am not familiar with the Pimoroni release testing process and do not want to make a further change if it prolongs the release of this PR. However, if it's as simple as "yeah looks good" from Pimoroni along with my own testing, then will happily adjust the PR to accommodate it. I can't do more without input from someone like @Gadgetoid but I feel like there might be some other larger issues in the raspberry pi space taking their attention at the moment, from other threads I've seen in the interwebs. |
|
fully understand and appreciate your comment @sjefferson99 I also see the bigger issues with the Pi5 and bookworm platforms taking up a huge effort, but this is the point of community contribution, to share the load and for a single individual to not be a bottleneck (no critique of pimironi individuals they can only do so much) |
Yeah, looks good! If you add the change here, then the CI should build a firmware that others can grab and test. Avoids having to wait for me to make an official release. I've built this branch against the latest version of our MicroPython firmware (v1.22.2 vs the old v1.20.4) for testing. IIRC this includes some additional WiFi fixes. You can find a .uf2 here - https://github.com/pimoroni/enviro/actions/runs/8783809878 The full list of changes is utterly exhaustive, though, but I'm putting it below for posterity:
|
Online check was incorrectly including state 3 (Connected with IP) as a condition to disconnect and retry, adjusted to include only state numbers below this.
|
@Gadgetoid @ikonia Branch updated and I can see reflected above in PR. |
|
Updated in my new-MicroPython PR also, builds should pop up here: https://github.com/pimoroni/enviro/actions/runs/8784459020/job/24102730608 |
|
amazing effort, thank you |
|
My strategy post merge is to tag a release against the current version of MicroPython, then merge my MicroPython bump and tag a new release against that. This should give us two firmwares that have:
So there's something to roll back to if the MicroPython bump causes any new problems. I don't imagine this being the case, but I'd rather be safe than sorry. @sjefferson99 thank you for your continued help and support here, I'm juggling quite a bit of software and it's dangerously easy to overlook things. |
|
Thankyou @Gadgetoid . I might actually get to use my enviro now if it stays up for more than 12 hours. I suppose I better hunt the box out again and bench test for a while. Fingers Crossed. |
|
The work that's been done in this thread, I've been testing for a few days from the CI build, and the difference is huge, stability, and oddly accuracy on the thermal sensor is improved along with minor stuff too. This is a big deal release, and it's very much appreciated |
Updates to the wifi connection code as originally presented by @mrexodia on #173
I have created a PR as this code addressed the board crashing issues I had mentioned in #87
Introduces better logging and network information, disconnects and retries connections with no IP address, applies some elements from the RP2040 manual on correctly configuring a wifi connection.
Critically, introduces a disconnect command after reading upload, which I believe is the part that fixes my board crashing issues.
I have adjusted the originally provided code to only disable power saving mode if on a USB supply, placed the wifi country in the config file and also replaced the custom logging function with the PHEW logging module for consistency.
I have tested correct behaviour on well behaved networks, also those that fail to give an IP and note correct disconnect and retry. I have also seen better performance in terms of no board lockups where they were guaranteed before after ~ 2 hours of frequent polling.