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

Add Weather Underground destination #165

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

sjefferson99
Copy link
Contributor

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.

@sjefferson99
Copy link
Contributor Author

Added rain per hour and rain today.
Merged rain per hour and day #169 and added these results to processing wunderground.py

@sjefferson99
Copy link
Contributor Author

Capture potential missed rain readings, see #169

@sjefferson99
Copy link
Contributor Author

Removed solar radiation TODO as values were far too low as a result of the luminance sensor being inside the Stevenson screen.

@sjefferson99
Copy link
Contributor Author

Merged local time adjustment from rain-per-hour-and-day to fix DST offset issue on weather underground accumulated rain today graph.

@sjefferson99
Copy link
Contributor Author

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.

@sjefferson99
Copy link
Contributor Author

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.

@sjefferson99
Copy link
Contributor Author

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.

@ZodiusInfuser ZodiusInfuser added the enhancement New feature or request label Jul 10, 2023
@MrDrem
Copy link

MrDrem commented Aug 29, 2023

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).
As per issue #70 rain is currently correctly reported as per the Met office reporting requirements.

Time adjustments and data concatenation should done on the reporting side, not on the data gathering side.

@sjefferson99
Copy link
Contributor Author

@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

@Gadgetoid Gadgetoid added this to the 1.0.0 milestone Apr 24, 2024
@sjefferson99
Copy link
Contributor Author

@Gadgetoid Rebased and includes the code from #159 and #169

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

Successfully merging this pull request may close these issues.

4 participants