Skip to content

python: Better error checking for events from python #270

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.

Sign up for GitHub

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

Open
rm5248 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

python: Better error checking for events from python #270

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 26 additions & 16 deletions python/osdp/peripheral_device.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from typing import Callable, Tuple

from .helpers import PDInfo, PDCapabilities
from .constants import LogLevel
from .constants import LogLevel, Event, CardFormat

class PeripheralDevice():
def __init__(self, pd_info: PDInfo, pd_cap: PDCapabilities,
Expand All @@ -32,9 +32,8 @@ def __init__(self, pd_info: PDInfo, pd_cap: PDCapabilities,
@staticmethod
def refresh(event, lock, ctx):
while not event.is_set():
lock.acquire()
ctx.refresh()
lock.release()
with lock:
ctx.refresh()
time.sleep(0.020) #sleep for 20ms

def _internal_command_handler(self, command) -> Tuple[int, dict]:
Expand Down Expand Up @@ -65,21 +64,33 @@ def get_command(self, timeout: int=5):
return cmd

def submit_event(self, event):
self.lock.acquire()
ret = self.ctx.submit_event(event)
self.lock.release()
return ret
# Do some sanity checks to make sure that the event has correct data
if not isinstance(event, dict):
raise TypeError("event must be of type dict")

if not 'event' in event:
raise TypeError("event does not contain 'event' key")

if event['event'] == Event.CardRead:
expected_keys = ["reader_no", "format", "direction", "data"]
Copy link
Member

@sidcha sidcha Dec 26, 2025

Choose a reason for hiding this comment

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

These are checked in osdp_sys and the missing keys are reported from there (see here). If this info is insufficient, the code there needs to be extended to avoid code duplication.

Copy link
Author

@rm5248 rm5248 Dec 30, 2025

Choose a reason for hiding this comment

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

It is checked there, but the problem is that the calling code overwrites the python exception here in pd.c. This leads to the unclear error message.

If you comment out that line in pd.c the proper exception is set.

My logic behind doing it in python is that it's easier to check for multiple keys and give the user a more clear error message before dropping to the C code.

Copy link
Member

@sidcha sidcha Jan 6, 2026

Choose a reason for hiding this comment

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

We can extend the the error reporting infrastructure to append the messages from lower layers. Will that solve this problem for you?

He C layer is in much better place to check and report these issues -- the low level methods check and report it from one place which means the check will not be outdated silently when new members are added to LibOSDP.

Copy link
Author

@rm5248 rm5248 Jan 6, 2026

Choose a reason for hiding this comment

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

We can extend the the error reporting infrastructure to append the messages from lower layers. Will that solve this problem for you?

He C layer is in much better place to check and report these issues -- the low level methods check and report it from one place which means the check will not be outdated silently when new members are added to LibOSDP.

I have no strong feelings one way or the other, if the messaging coming back from the C code is better that is fine with me.

if event['format'] != CardFormat.ASCII:
expected_keys.append("length")
if not all(key in event for key in expected_keys):
raise TypeError("cardread event missing expected key")

with self.lock:
ret = self.ctx.submit_event(event)
return ret

def notify_event(self, event):
from warnings import warn
warn("This method has been renamed to submit_event", DeprecationWarning, 2)
return self.submit_event(event)

def register_file_ops(self, fops):
self.lock.acquire()
ret = self.ctx.register_file_ops(0, fops)
self.lock.release()
return ret
with self.lock:
ret = self.ctx.register_file_ops(0, fops)
return ret

def is_sc_active(self):
return self.ctx.is_sc_active()
Expand Down Expand Up @@ -108,10 +119,9 @@ def start(self):
self.thread.start()

def get_file_tx_status(self):
self.lock.acquire()
ret = self.ctx.get_file_tx_status(0)
self.lock.release()
return ret
with self.lock:
ret = self.ctx.get_file_tx_status(0)
return ret

def stop(self):
if not self.thread:
Expand Down