-
Notifications
You must be signed in to change notification settings - Fork 64
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
Don't thow an error when string can't be decoded to ascii #170
Conversation
Removed import
Don't yield anything on error, just pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but maybe we should add some logging? Else it silently fails and you wouldn't get a any signals of it doing so
dsmr_parser/clients/serial_.py
Outdated
try: | ||
decoded_data = data.decode('ascii') | ||
except: | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe put a logger statement here:
logger.warning('Failed to decode telegram data: %s', e)
dsmr_parser/clients/serial_.py
Outdated
try: | ||
decoded_data = data.decode('ascii') | ||
except: | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe put a logger statement here:
logger.warning('Failed to decode telegram data: %s', e)
dsmr_parser/clients/serial_.py
Outdated
try: | ||
decoded_data = data.decode('ascii') | ||
except: | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe put a logger statement here:
logger.warning('Failed to decode telegram data: %s', e)
Makes sense, tough I'd consider outputting |
Alright the python 3.7 doesnt seem to run, but your change looks good. Thanks! I will try to make a new build soon! |
Possible solution for Issue #80. Instead of throwing an exception when input from serial can't be parsed, it passes the current step while the loop keeps going. This will mean one 'row' is missing from the telegram. The user should keep this in mind and not expect every telegram to be formed the same way.
Another option is to yield None, which I tried first. On second thought, it is not that helpful because there is nothing to return (though we could give the 'raw' bytestring, but that would complicate things for the user as well.