-
Notifications
You must be signed in to change notification settings - Fork 86
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
Add Weather Underground destination #165
base: main
Are you sure you want to change the base?
Conversation
Added rain per hour and rain today. |
Capture potential missed rain readings, see #169 |
Removed solar radiation TODO as values were far too low as a result of the luminance sensor being inside the Stevenson screen. |
Merged local time adjustment from rain-per-hour-and-day to fix DST offset issue on weather underground accumulated rain today graph. |
After experiencing some memory issues and adding a lot of logging, I've found that while the for loop to build the URL consumes a fair chunk of memory, this is automatically released as needed even mid for loop (interestingly gc.collect() added manually does nothing, have to rely on automatic collection). The issue comes at urequests.get(url), this seems to consume around 35Kb of RAM and we never get it back until board reset, which limits cached upload count to around 4 at best. As gc.collect() doesn't appear to be doing anything, I can't even workaround this with liberal collection, it will need more investigation. This doesn't cause an issue in itself beyond a delay in uploads as the boards enters upload failure sleep and the RTC reset frees all the RAM for the next attempt. However, I have a new issue where the board crashes after a few hours with a memory allocation failure in the log truncation function. It isn't clear if this is exacerbated by the poor memory management of the wunderground upload or potentially related to my recent upgrade to v1.19.17 firmware which subsequently reported massive memory issues, but this still occurred on v1.19.18 which claimed to address these. I am going to make sure I am running just the weather underground branch, drop back to v1.19.16 and see if my memory error still presents. In parallel I will try and fix the urequests memory use which will need fixing either way. |
The weather station has been running without issue on v1.19.16 since my last comment, so I'm pretty confident the memory allocation issues were enviro firmware v1.19.17+ related. Just the memory consumption on request send to address so that more than 3 or 4 cached readings can be sent. |
Forgot to add request.close() at the end of the result send function. This caused the memory leak as you would expect. @ZodiusInfuser This PR now works as expected for me on enviro firmware v1.19.16. |
I strongly recommend that this is not pulled as is. As per my comments on #169 : BST should not be used for reporting times, times should be kept constant (so UTC is ideal as now, but otherwise GMT, or local equivalent). Time adjustments and data concatenation should done on the reporting side, not on the data gathering side. |
@MrDrem As per my last comment on #169, there is no intent to incorporate BST into the reading timestamps, but the rain per day reading has to be calculated prior to submission for this destination and BST is required to determine local midnight for the rain/day reading to be correct, despite being submitted in UTC. Please let me know if you are seeing anything incorrect from the intent as per #169 |
1a2b160
to
34504bb
Compare
@Gadgetoid Rebased and includes the code from #159 and #169 |
As requested in #38 added destination for Weather Underground (WU/wunderground).
This is based on and tested on the weather board as this is all I have, however in theory it will grab any matching sensor output from other boards and upload these. However, as the rest will be silently dropped, it is probably best to only use this destination for weather boards.
Rain and luminance are not yet included as this requires conversion work and rain tracking improvements, but TODOs are placed appropriately.
Included a documentation update spelling out the above and instructions to configure. There is also an updated provisioning wizard to include WU.
As WU expects sea level adjusted pressure I have merged #159 into this PR and will attempt to keep them updated while they are both open. There is no provisioning for sea level pressure as I am unsure how best to integrate into the existing design (and my html forms knowledge is very rusty), but if this is pushed forward in #159 I will merge into this PR as well. It is possible to manually configure sea level pressure in the config file and defaults to enabled = False. Having sea level pressure disabled will simply drop the observed pressure reading from the WU upload.