pa1010d-python icon indicating copy to clipboard operation
pa1010d-python copied to clipboard

Update __init__.py

Open elschnorro77 opened this issue 3 years ago • 1 comments
trafficstars

fix _write_sentence for pyhton3 fix send_command for pyhton3 add missing elements from nmea sentences add initialisation of PA1010D add waitforfix

elschnorro77 avatar Feb 01 '22 13:02 elschnorro77

The missing NMEA sentences are handy, thank you, but I have a few concerns;

  1. Is the fix to _write_sentence just swallowing IOErrors?
  2. This is a lot of logging/error swallowing, what are you trying to achieve here and why?

This level of exception handling is a bit heavy-handed for the underlying library. If you need to catch exceptions in read/write that could be done in the client app.

Generally:

  1. Your try/catch should be around one or two lines of relevant code- as it stands the change of indentation across multiple lines of code makes it near impossible to tell what's actually changed
  2. It's redundant to catch an exception and just raise it again
  3. It's messy to catch an exception and log it at the library level, makes development iteration more difficult. If a user is concerned with logging they can add that over the top. Maybe a hardened example is the best way to handle this?
  4. If IO Errors are an intermittent problem in your setup (sometimes it's unavoidable because i2c is the devil) it might be worth having some kind of retry-on-fail, specially for writes

If you'd be happy to strip out all the exception handling for now and make this a clean PR just adding the missing NMEA sentences, we can go from there with some kind of logging example.

Gadgetoid avatar Feb 04 '22 11:02 Gadgetoid