Major patch to fix a lot of the issues with Enviro

[ZodiusInfuser]

This is a work in progress still, but most of the rain sensor issues look to be fixed now

[PascalKern]

Hi @ZodiusInfuser

Hope you are fine. My I ask something about one commit included in this merge?

I realized that you removed the usage of phew in enviro/__init__.py:connect_to_wifi() with the commit: 60d29ea.

Would you mind to explain quickly what the reason was? I'm just wondering as the old code did lock way cleaner and I (personally) do not like code duplication. ;)
But on the other side, these functions match even not for the phew project as it feels not like part of a webserver.

I'm just asking because I'm still checking and playing around with the lost voltage measuring beside the rain issue (which seems to be gone - at the moment) and don't want to invest in a part of the code which will change anyway sooner than later.

Looking forward to any feedback and info around mentioned commit and changes. But no hurry - I still have currently not a lot of time anyway 🙈

I wish you the best and a lovely holiday season with soon nice festive days 🧑‍🎄

Best,
Pascal

[ZodiusInfuser]

Hi @PascalKern,

It's not so much that I removed the usage of phew in enviro/__init__.py:connect_to_wifi(), but rather did not get around to re-adding it. As mentioned in the v0.0.9 release notices, I rolled back to v0.0.2 and slowly reapplied changes from v0.0.8 back on to it.

The lack of this change in v0.0.9 was a combination of not being sure the two code sections were equivalent, and the release already having a lot of changes in it that I needed to get into the hands of users. So, it is possible that the change gets re-applied at some point in the future, but it is low down on my priority list. I agree that removing code duplication is preferred, so if you are open to testing the two versions and giving your thoughts, then that would be most welcome.

Hope that clarifies. Happy Holidays to you too.

P.S If you do look at the battery thing, note that it's a far bigger rabbit hole than it appears. I've tried many things suggested online for reading VSYS voltage of the Pico W, but they all cause complications with the WiFi in some way.

[PascalKern]

Hi @ZodiusInfuser

Thanks a lot for your reply and the clarification. I hadn't the full picture of your work ie. v0.0.2 to v0.0.8 but of course, I’m well aware that the merge was quite huge and had to become available in the wild. Too many fixes were awaited out there. 😊

Also totally understand the decision in such a situation with two maybe equivalent code portions. Especially as it works well now and the removal of the duplication has time as of its more „cosmetic“ aspect.

When I find some time I‘ll test the „two ways“. 🤠

One last question regarding the voltage. I‘m well aware that this can be/is quite a rabbit hole but still…. I like rabbits! 😀
My question: did you test these approaches with different boards or maybe mainly with the weather? I‘m asking because I only had issues with my weather board but not with the indoor, grow or urban while the voltage measurement was still implemented.

Again. Many thanks for your time and the information. Have a nice time and keep up the good work. Thanks a lot.

Best,
Pascal

[ZodiusInfuser]

For the battery monitoring I was mainly testing on Weather but did try the others and got user reports from the other boards too. Reading the voltage was fine. The issue was doing it in a way that would not cause the WiFI to lock-up. This was most prevalent in this issue: #78 which made it virtually impossible to debug the board or get your files off it. As soon as I added the battery monitoring code as part of my patching of 0.0.8 onto 0.0.2, I got that issue again.

# read battery voltage - we have to toggle the wifi chip select
# pin to take the reading - this is probably not ideal but doesn't
# seem to cause issues. there is no obvious way to shut down the
# wifi for a while properly to do this (wlan.disonnect() and
# wlan.active(False) both seem to mess things up big style..)
old_state = Pin(WIFI_CS_PIN).value()
Pin(WIFI_CS_PIN, Pin.OUT, value=True)
sample_count = 10
battery_voltage = 0
for i in range(0, sample_count):
  battery_voltage += (ADC(29).read_u16() * 3.3 / 65535) * 3
battery_voltage /= sample_count
battery_voltage = round(battery_voltage, 3)
Pin(WIFI_CS_PIN).value(old_state)

Since then I have tried other methods of capturing the voltage following these sources, but still encountered issues:
https://forums.pimoroni.com/t/pico-w-not-reading-gpio24/20510
https://www.reddit.com/r/raspberrypipico/comments/xalach/measuring_vsys_on_pico_w/
https://forums.raspberrypi.com/viewtopic.php?t=339994#p2036710

One workaround idea we've had internally (which would only work for Enviro and on battery) would be to introduce a single battery read as part of the boot process, in the same way we do for setting the HOLD_VSYS_EN pin high. I am not sure how to do that yet, but since you like rabbits, here are the files you'd need to modify ;)
https://github.com/pimoroni/pimoroni-pico/blob/main/micropython/_board/picow_enviro/wakeup_gpio.patch
https://github.com/pimoroni/pimoroni-pico/blob/main/micropython/modules/wakeup/wakeup.cpp
Note that first one applies a patch onto the Pico SDK itself. If you want to test, you'd be best to take advantage of our github actions that auto-generate the relevant enviro micropython build (it can be done locally but is a faff).

Also, it may be that this point is too early to get a usable reading, as the power rail may not have stabilised.

If you do attempt this, good luck! No pressure though!

[PascalKern]

Hi @ZodiusInfuser

Finally!

You might have seen my PR but just in case. The rabbit hole wasn't that deep as I got very lucky in my initial recherche(s) by finding the solution as described in my PR (Re-Enable the battery monitoring on Enviro! 🥳 #146)

I just needed a bit too much time to implement and test it but here we go.

Just wanted to thank you for your info in the last comment. Let's see what you and your internal guys think of this solution as it does not need to thinker with the wakeup implementation at least not that I experienced it thanks to the solution provided by Daniel Perron and his solution for his PicoWSoloar project.

Best,
Pascal