gjr80 / weewx-realtime_gauge-data

Near realtime support for updating of SteelSeries Weather Gauges by WeeWX
GNU General Public License v3.0
9 stars 5 forks source link

Add rsync support. Add precise pressure and pressure trend fields. #19

Closed chaunceygardiner closed 4 years ago

chaunceygardiner commented 4 years ago

Hi Gary,

I’ve been running a modified copy of your rtgd.py file for about nine months or so. Besides using the gauges, I have a modified Seasons skin that also updates every two seconds via the gauge-data.txt file. You can see it in action at www.paloaltoweather.com. That production version is WeeWX 3.9.2 (modified). That website is on a Google GCE instance with gauge-data.txt being rsync’d to it every two seconds.

I got a bunch of changes in the WeeWX development branch for 4.x. Those changes included a change to the rsync utility so that it would work with a single file. Now that that change is imminent (with the upcoming 4.x release), I’m hoping to get the rsync ability included in rtgd.py.

The change allows one to specify an rsync server. Furthermore, with the default values, the rsyncs have been very good at keeping up. For example, as I write this, it’s nearly the end of the day, and logwatch reports the following:

rsync: files uploaded 26096 rsync: gauge-data: connection timeouts 6

That is, uploading gauge-data.txt every two seconds (the files uploaded also includes the skins sync), the connection only timed out 6 times and there were no write timeouts. It is set to discard any data older than 4 seconds, but there were none discarded today.

In addition to the rsycn capability, this change includes two new fields in gauge-data.txt. These two fields are more precise (more decimals) for pressure and pressure trend. They are used to update the seasons page.

Thank you for considering this change.

Cheers, John

gjr80 commented 4 years ago

Hi John,

Thanks, unfortunately I am not as organised as Tom K when it comes to keeping on top of PRs!

Rsync will be a good addition, I seem to remember rolling my own rsync either for rtgd or one of my other similar extensions and didn't really like the result. This looks fine though.

When it comes to adding extra fields my first thought is be wary of bloating the output. I had one other chap wanted to add a few extra fields and the approach I decided to take was to introduce an optional field map that allowed the user to specify additional fields in the json output if required. I wonder if may that approach could be extended to allow the user to override formats (well number of decimal places really) of any/all fields. So, for example, you could specify that you want press to be output as #.4f instead of the default #.3f and you could specify presstrendval as #.6f. This is a little different to the standard WeeWX approach which is to format on a per unit basis rather than per field, but I don't see an issue with that. This keeps the field count the same and I don't believe the SteelSeries Gauges code would have a problem with the extra decimal places. Would that sort of arrangement suit you? I guess you would need to update Seasons based on presstrendvalue rather than presstrendvaluePRECISE but I expect that is what you would have preferred from the start.

What I would like is a default install with minimal config to be generating a compact and valid gauge-data.txt but I am more than happy to give users the ability to extend/change that output as required.

I have a WeeWX v4 branch sitting ready for release when WeeWX 4 hits the street, will certainly make sure these changes are rolled into that release.

Gary

chaunceygardiner commented 4 years ago

Hi Gary,

I much appreciate you looking at the request and I fully understand why you don’t want to take the two extra fields. I wish I had not put them in the same pull request.

You didn’t say what you’d like me to do WRT the PR. I think you can just remove those lines on your side. Or do you want me to send a new PR?

As for your suggestion on how you would handle the extra precision, it sounds great to me. I’d love to drop using the PRECISE versions. It sounds like you are doing that part, correct? I’m not trying to hurry you. I’m just clarifying that you aren’t looking for another PR from me.

Again, thanks for your time.

Cheers, John

gjr80 commented 4 years ago

John,

OK, now that I have things straight in my head how about this for a way ahead:

Can you remove the extra precision lines from your PR, unless I am wrong that is just four lines of code. That will leave just rsync in the PR. I will then accept that PR and we have a WeeWX v4 compatible release ready to go for when Tom pushes out v4 (was talk of this weekend but think we have missed that). (If I have this wrong and this entails a whole pile of work for you let me know and I will manually pull in the rsync code only and knock the PR on the head)

I will create a new branch off master and work on allowing extra precision and a few other things I have in mind. Will be a bit more involved than adding a couple of extra fields so might take a few days but we have a few rainy days forecast here so with a bit of luck that should be plenty of time for me to get things working. When the time comes I can just merge that branch back into master.

Didn't think you were hurrying me at all, in fact every now and then my enthusiasm for WeeWX wanes somewhat, these changes have lifted my enthusiasm level so thanks.

Gary

chaunceygardiner commented 4 years ago

I’m going to close this PR and send another without the pressure and pressure trend precision changes.