jardiamj / wmII

GNU General Public License v3.0
4 stars 2 forks source link

Update wmII.py #9

Closed pe2kmv closed 2 years ago

pe2kmv commented 2 years ago

Changed the function genLoopPackets() by adding try..except condition to catch errors. Instances where no (valid) response was received from the WM2 the function get_readings() caused an unexpected exit which crashed weewx.

jardiamj commented 2 years ago

Do you have a log of what error caused the weewx crash? The Station's get_readings_with_retry method is meant to take care of invalid packets received sporadically from the WMII Station. My station has been running for a few years without any issues and I don't think having a general try...except in that loop is a good idea. At least, we should have an exception type there and log the error.

JayBee103 commented 2 years ago

I too have been running this for several years without issue.

I have not upgraded weewx in that time in the spirit of "if it ain't broke, don't fix it" but I am interested to know if this is under the newer release of weewx?

Jim

On Thu, Dec 30, 2021, 11:14 PM Jardi Martinez @.***> wrote:

Do you have a log of what error caused the weewx crash? The Station's get_readings_with_retry method is meant to take care of invalid packets received sporadically from the WMII Station. My station has been running for a few years without any issues and I don't think having a general try...except in that loop is a good idea. At least, we should have an exception type there and log the error.

— Reply to this email directly, view it on GitHub https://github.com/jardiamj/wmII/pull/9#issuecomment-1003272022, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGH2ZJ36RMR645A2U2IGJJTUTU3Z7ANCNFSM5KLXNHQA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

pe2kmv commented 2 years ago

I've rolled back my change in wmII.py to capture some data in the syslog:

Jan 1 21:08:18 questor weewx[385] CRITICAL main: Caught unrecoverable exception:

Jan 1 21:08:18 questor weewx[385] CRITICAL main: **** ord() expected a character, but string of length 0 found

Jan 1 21:08:18 questor weewx[385] CRITICAL main: **** Traceback (most recent call last):

Jan 1 21:08:18 questor weewx[385] CRITICAL main: **** File "/usr/share/weewx/weewxd", line 157, in main

Jan 1 21:08:18 questor weewx[385] CRITICAL main: **** engine.run()

Jan 1 21:08:18 questor weewx[385] CRITICAL main: **** File "/usr/share/weewx/weewx/engine.py", line 208, in run

Jan 1 21:08:18 questor weewx[385] CRITICAL main: **** for packet in self.console.genLoopPackets():

Jan 1 21:08:18 questor weewx[385] CRITICAL main: **** File "/usr/share/weewx/weewx/drivers/wmII.py", line 116, in genLoopPackets

Jan 1 21:08:18 questor weewx[385] CRITICAL main: **** readings = self.station.get_readings_with_retry(

Jan 1 21:08:18 questor weewx[385] CRITICAL main: **** File "/usr/share/weewx/weewx/drivers/wmII.py", line 302, in get_readings_with_retry

Jan 1 21:08:18 questor weewx[385] CRITICAL main: **** buf = self.get_readings()

Jan 1 21:08:18 questor weewx[385] CRITICAL main: **** File "/usr/share/weewx/weewx/drivers/wmII.py", line 288, in get_readings

Jan 1 21:08:18 questor weewx[385] CRITICAL main: **** if ord(c) != 1:

Jan 1 21:08:18 questor weewx[385] CRITICAL main: **** TypeError: ord() expected a character, but string of length 0 found

Jan 1 21:08:18 questor weewx[385] CRITICAL main: **** Exiting.

To trap this error I've inserted the try-except routine in genLoopPackets. Since that mod weewx with the wmII.py driver runs flawlessly (see https://www.pe2kmv.nl/weewx)

Kind regards,

Ronald.

Op vr 31 dec. 2021 om 06:14 schreef Jardi Martinez @.***>:

Do you have a log of what error caused the weewx crash? The Station's get_readings_with_retry method is meant to take care of invalid packets received sporadically from the WMII Station. My station has been running for a few years without any issues and I don't think having a general try...except in that loop is a good idea. At least, we should have an exception type there and log the error.

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you authored the thread.Message ID: @.***>

jardiamj commented 2 years ago

Thanks for the log, this is very helpful. The source of this bug is a change in the return type of pyserial's read method, when I wrote the driver it would return a string but newer versions return a byte string when available (like in Python 3), which is what the newest weeWX version uses: https://pyserial.readthedocs.io/en/latest/pyserial_api.html

That causes the check in line 286 to fail because an empty byte string evaluates to false in if c == "". I fixed it by replacing that with if not c which should work for both empty strings and byte strings.

I will push the change right now. Can you check if that fixes the issue? if so, I will close this PR.

Thanks for taking the time to get the log messages.

Jardi.

pe2kmv commented 2 years ago

Hi Jardi,

Thanks for your action. I've pulled the update this morning and the entire day the system has ran smoothly. So the issue can be closed.

Best regards, Ronald.

Op ma 3 jan. 2022 om 06:51 schreef Jardi Martinez @.***

:

Thanks for the log, this is very helpful. The source of this bug is a change in the return type of pyserial's read method, when I wrote the driver it would return a string but newer versions return a byte string when available like in Python 3, which is what the newest weeWX version uses.

This causes the check in line 286 to fail because an empty byte string evaluates to false in if c == "". I fixed it by replacing it with if not c which should work for both empty strings and byte strings.

I will push that change right now. Can you check if that fixes the issue? if so, I will close this PR.

Thanks for taking the time to get the log messages.

Jardi.

— Reply to this email directly, view it on GitHub https://github.com/jardiamj/wmII/pull/9#issuecomment-1003883342, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACJXCSMYAOW4ZPRL5IG7KRLUUE2P3ANCNFSM5KLXNHQA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

jardiamj commented 2 years ago

Closing PR since the bug could be fixed without this try...except. https://github.com/jardiamj/wmII/pull/9#issuecomment-1003883342