Closed randallgray closed 4 years ago
Thank you Jardi for replying to my request. I agree that the functions can stay even though they are no longer being used -- I appreciate the logic of your code and am glad it's still there to see and learn from.
Obviously, I didn't read that last part of the code close enough -- sorry for suggesting something so stupid.
Just want to say thank you for what you've done so far -- it helped tremendously having something to start with.
rg
On Wed, Dec 11, 2019 at 7:11 PM Jardi Martinez notifications@github.com wrote:
@jardiamj requested changes on this pull request.
Hi, thanks for the PR. I haven't done some needed clean up in this driver. This get_average() function is no longer needed neither does the get_average_direction() inside the WindGauge class; they can be safely removed. I wrote those functions when I started writing the driver because I originally it averaging the wind speed and direction over a time interval. After asking in the forums, Thomas let me know that it was not recommended for a driver to be doing extra calculations because weeWX already has some Accumulator functions that do that. So, after experimenting a little bit I decided to change the driver to just sampling the wind direction at every interval but I didn't remove those functions even though they are not being used. I doesn't hurt to leave the functions there, but if you indent the get_average() function you would also have to change line 303 from: return get_average(data) to: return self.get_average(data) Because then the get_average() function would belong to the WindGauge class.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jardiamj/BYOWS_RPi/pull/4?email_source=notifications&email_token=AA7LMK73MP2TVMZWQTYGT53QYGTXFA5CNFSM4JZU2I42YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCO5DXSY#pullrequestreview-330972107, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA7LMK62EIZICZJEPHZQYMDQYGTXFANCNFSM4JZU2I4Q .
The submitted changes allow for Python3 compatibility. This will allow you to run the driver under WeeWX 4.x. Currently, it's been tested and functions with WeeWX 4.0.0b5.
The sensors I'm using all rely on Python3 libraries which only run under 4.x and Thomas is no longer developing for 3.x so upgrading seems like a good idea anyhow...
One other thing to mention -- I have not yet verified the backward compatibility with Py 2.7.x. My hope is that it will work with either.
Thank you!