Skip to content

Commit

Permalink
enhance exception handling for ERR_LOG and ERR_IGNORE
Browse files Browse the repository at this point in the history
  • Loading branch information
@semuadmin
semuadmin committed May 18, 2024
1 parent a23b601 commit 8fa0bcf
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 34 deletions.
61 changes: 29 additions & 32 deletions src/pyubx2/ubxreader.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,13 @@ def read(self) -> tuple:
:return: tuple of (raw_data as bytes, parsed_data as UBXMessage, NMEAMessage or RTCMMessage)
:rtype: tuple
:raises: UBXParseError (if unrecognised protocol in data stream)
:raises: Exception (if unrecognised protocol in data stream)
"""

flag = True
parsing = True
while parsing: # loop until end of valid message or EOF
try:

try:
while flag: # loop until end of valid message or EOF
raw_data = None
parsed_data = None
byte1 = self._read_bytes(1) # read the first byte
Expand All @@ -158,7 +158,7 @@ def read(self) -> tuple:
# if protocol filter passes UBX, return message,
# otherwise discard and continue
if self._protfilter & UBX_PROTOCOL:
flag = False
parsing = False
else:
continue
# if it's an NMEA message (b'\x24\x..)
Expand All @@ -167,7 +167,7 @@ def read(self) -> tuple:
# if protocol filter passes NMEA, return message,
# otherwise discard and continue
if self._protfilter & NMEA_PROTOCOL:
flag = False
parsing = False
else:
continue
# if it's a RTCM3 message
Expand All @@ -177,36 +177,32 @@ def read(self) -> tuple:
# if protocol filter passes RTCM, return message,
# otherwise discard and continue
if self._protfilter & RTCM3_PROTOCOL:
flag = False
parsing = False
else:
continue
# unrecognised protocol header
else:
if self._quitonerror == ERR_RAISE:
raise UBXParseError(f"Unknown protocol {bytehdr}.")
if self._quitonerror == ERR_LOG:
return (bytehdr, f"<UNKNOWN PROTOCOL(header={bytehdr})>")
continue

except EOFError:
return (None, None)
except (
UBXMessageError,
UBXTypeError,
UBXParseError,
UBXStreamError,
nme.NMEAMessageError,
nme.NMEATypeError,
nme.NMEAParseError,
nme.NMEAStreamError,
rte.RTCMMessageError,
rte.RTCMParseError,
rte.RTCMStreamError,
rte.RTCMTypeError,
) as err:
if self._quitonerror:
self._do_error(err)
parsed_data = str(err)
raise UBXParseError(f"Unknown protocol header {bytehdr}.")

except EOFError:
return (None, None)
except (
UBXMessageError,
UBXTypeError,
UBXParseError,
UBXStreamError,
nme.NMEAMessageError,
nme.NMEATypeError,
nme.NMEAParseError,
nme.NMEAStreamError,
rte.RTCMMessageError,
rte.RTCMParseError,
rte.RTCMStreamError,
rte.RTCMTypeError,
) as err:
if self._quitonerror:
self._do_error(err)
continue

return (raw_data, parsed_data)

Expand Down Expand Up @@ -346,6 +342,7 @@ def _do_error(self, err: Exception):
raise err from err
if self._quitonerror == ERR_LOG:
# pass to error handler if there is one
# else just print to stdout
if self._errorhandler is None:
print(err)

This comment has been minimized.

Sign in to view

Copy link
@carstenandrich

carstenandrich May 19, 2024

IMHO a library should never print to stdout, as you never know if the application using the library is using stdout to pipe precious data to another process or write it to a file. An imported library writing to stdout would clobber this data. If you really have to write error messages, use stderr, but I would still frown upon that.

Better solution, is to use the logging module, which defaults to printing to stderr, when not configured. See comment below for details.
else:
Expand Down
7 changes: 5 additions & 2 deletions tests/test_stream.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ def testUBXITERATE_ERR4(
self.assertEqual(EXPECTED_RESULT, res)

def testBADHDR_FAIL(self): # invalid header in data with quitonerror = 2
EXPECTED_ERROR = "Unknown protocol b'\\xb5w'"
EXPECTED_ERROR = "Unknown protocol header b'\\xb5w'"
with self.assertRaises(UBXParseError) as context:
i = 0
with open(os.path.join(DIRNAME, "pygpsdata-BADHDR.log"), "rb") as stream:
Expand All @@ -551,11 +551,14 @@ def testBADHDR_FAIL(self): # invalid header in data with quitonerror = 2

def testBADHDR_LOG(self): # invalid header in data with quitonerror = 1
i = 0
self.catchio()
with open(os.path.join(DIRNAME, "pygpsdata-BADHDR.log"), "rb") as stream:

ubr = UBXReader(stream, quitonerror=ERR_LOG)
for raw, parsed in ubr:
i += 1
self.assertEqual(parsed, "<UNKNOWN PROTOCOL(header=b'\\xb5w')>")
output = self.restoreio().split("\n")
self.assertEqual(output, ["Unknown protocol header b'\\xb5w'."])

This comment has been minimized.

Sign in to view

Copy link
@carstenandrich

carstenandrich May 19, 2024

When using logging to write the error message, unittest.TestCase.assertLogs is a cleaner solution to test the error message than capturing stdout.

def testBADHDR_IGNORE(self): # invalid header in data with quitonerror = 0
i = 0
Expand Down

1 comment on commit 8fa0bcf

@carstenandrich
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick git diff to use logging instead of printing to stdout. Worked for the, but didn't do the unittests.

diff --git a/src/pyubx2/ubxreader.py b/src/pyubx2/ubxreader.py
index cf8294a..5f3b979 100644
--- a/src/pyubx2/ubxreader.py
+++ b/src/pyubx2/ubxreader.py
@@ -21,6 +21,7 @@ Created on 2 Oct 2020
 """
 
 from socket import socket
+import logging
 
 import pynmeagps.exceptions as nme
 import pyrtcm.exceptions as rte
@@ -101,6 +102,7 @@ class UBXReader:
         self._labelmsm = labelmsm
         self._msgmode = msgmode
         self._parsing = parsing
+        self._logger = logging.getLogger(__name__)
 
         if self._msgmode not in (GET, SET, POLL, SETPOLL):
             raise UBXStreamError(
@@ -344,7 +346,7 @@ class UBXReader:
             # pass to error handler if there is one
             # else just print to stdout
             if self._errorhandler is None:
-                print(err)
+                self._logger.error(err)
             else:
                 self._errorhandler(err)
 
@@ -446,5 +448,5 @@ class UBXReader:
             if quitonerror == ERR_RAISE:
                 raise UBXParseError(errmsg) from err
             if quitonerror == ERR_LOG:
-                print(errmsg)
+                self._logger.error(errmsg)
             return None

Please sign in to comment.