grebleem / weewx-weatherlinkliveudp

Weewx Driver with UDP
6 stars 4 forks source link

Interested in python2? #17

Closed kk7ds closed 3 years ago

kk7ds commented 3 years ago

Hi,

I hate to be that guy, but I'm currently still running weewx on a machine under python2 (which is still supported by weewx, AFAIK). This is mostly because the distro I'm on is missing one package on python3 needed to run the python3 version and I'm trying to keep it fairly clean. Since I need to be able to receive UDP from the WLL, I'd rather not run this under docker and deal with an additional layer of network stuff.

I've tweaked the module to run under python2, which was mostly trivial - just removal of f-strings and a few other things. I'm curious, would you accept a patch to make it python2 compliant for those of us still stuck there?

grebleem commented 3 years ago

Hi,

First of all apologies for the late response. Since January 2020 Python 2 is 'officially' dead. So I'm not sure about patching it to version 2. Although I think you are right that the most 'used' v3 are the f-strings. But to suite the v2.7 users I can make it a separate branch and I will try to keep it up to date as much as possible. Can you post the source files?

kk7ds commented 3 years ago

I'm a python programmer by day, so I'm very aware of the sunset and how much trouble it has caused for the industry :) In this case, I'm running on a new enough system, that system just doesn't have one of weewx's python3 dependencies packaged in deb form, so it won't actually install.

Maintaining a separate branch doesn't seem worth it, and is likely to just get ignored. I figured that since f-strings are non-critical and otherwise the code is compliant, it'd be good to maintain compatibility with python2 at least as long as weewx itself does. But, it's fine, I'll just keep maintaining my local copy until I can get my stuff moved far enough forward.

Thanks anyway!

grebleem commented 3 years ago

I don't have many experience with v2.7. I replaced all the f-strings, but is there also a change in the json decoder?

kk7ds commented 3 years ago

No json changes needed that I've noticed. There's a change needed because of some datetime differences (no datestamp() method in py2). So I added this:

def total_seconds(dt):
    return (dt - datetime.datetime(1970, 1, 1)).total_seconds()

For this:

     def calculate_rain(self):
-        if self.davis_date_stamp.timestamp() > self.rainbarrel.previous_date_stamp.timestamp():
+        if total_seconds(self.davis_date_stamp) > total_seconds(self.rainbarrel.previous_date_stamp):
             logdbg(self.current_davis_data)

Although I think you can just compare the datetimes directly. I was just trying to make it same-for-same in my copy.

Other than that, I had to remove the non-ASCII characters from the comments (i.e. the degree symbols, etc) in order to get regular python2 to parse the source fine (which assumes ASCII).

grebleem commented 3 years ago

Thank you for the information. I was a bit confused about the unicode representation u'{ when debugging the json data, but the functionality remains the same. I have rewritten the bugfix_september20 branch and tested with Python 2.7.18 (under pyenv) and it looks good. Perhaps you can give it a try. I will try to keep it compatible with Python 2 as long as possible, or if weewx decides to let go of it.

grebleem commented 3 years ago

Fixed in v0.2.9.