Skip to content

Driver error handling: real exceptions, no hangs - #98

Merged
sequoiap merged 5 commits into
masterfrom
fix/driver-errors
Oct 5, 2026
Merged

sequoiap merged 5 commits into
masterfrom
fix/driver-errors

Conversation

@sequoiap

@sequoiap sequoiap commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator

Fixes #88, #58, #60, #57, #79, #41. There is one commit per issue (#88 and #58 share one).

ppcl55x (#88, #58, part of #60)

  • Errors. A failed read or bad checksum raised CommunicationError, which was never imported, so it became a NameError. A timeout returned the in-band value (0xFF, 0xFF, 0xFF, 0xFF). Now:
    • a timeout raises TimeoutError;
    • a failed read raises OSError, chained to the serial error;
    • a bad checksum raises OSError.
  • Why built-in exceptions, not Pyro's CommunicationError (which ppcl55x raises CommunicationError without importing it, so read errors become NameError #88 suggested):
    • When a remote method raises a Pyro CommunicationError, the client's proxy drops its connection (Pyro5/client.py, _pyroInvoke). That releases any lock the client holds.
    • Pyro can only rebuild built-in exceptions (and its own) on the client. A PyroLab or pyserial exception class would reach the client as a SerializeError.
    • A test checks that both exceptions survive the trip through Pyro's serializer.
  • The driver hung for good after one failure. This wasn't in the issues, but the _read returns a magic (0xFF, 0xFF, 0xFF, 0xFF) tuple to signal a timeout #58 fix needs it.
    • _communicate created its threading.Lock() fresh on every call, so the lock serialized nothing.
    • Each call took a ticket and spun until its ticket was first in the queue. A call that raised never removed its ticket, so every later call spun forever.
    • That already happened on a bad checksum, and with _read returns a magic (0xFF, 0xFF, 0xFF, 0xFF) tuple to signal a timeout #58 it would happen on every timeout too. A single per-laser lock replaces both.
    • The wait for a reply now sleeps between polls instead of spinning the CPU.
  • on() and off() still tolerate unanswered no-ops. They send ten no-ops purely as a delay. With the old timeout value, an unanswered one passed silently, so it still does. The first command (Resena) isn't tolerated, so a laser that doesn't respond still makes on() fail.

Other bare except: clauses (#60)

  • tsl550 wavelength_logging: the while True never repeated, and every failure became the same RuntimeError.
    • A short read, which is what a serial timeout produces, now raises TimeoutError, saying how far it got ("after 1 of 3 points").
    • A serial error now raises OSError, chained to the original.
  • uc480, kdc101 and the nameserver: each now catches specific exceptions (or Exception), and kdc101 logs the traceback.

Arduino (#57)

analog_read spun at full CPU until a pin reported, forever if it never did. It now takes timeout: float = 1.0, sleeps between polls, and raises TimeoutError.

RTO (#79)

  • connect() no longer discards its VISA ResourceManager. It is kept, and close() closes it even if closing the device fails.
  • The timeout docstring said the default is 1 ms; it is 1000 ms (one second).

Motion examples (#41)

  • All six examples now construct the instrument with no arguments and call connect(serialno=..., ...), using the drivers' real keyword names. They close the stage in finally.
  • They no longer call os.add_dll_directory for the Kinesis DLLs, because thorlabs_kinesis adds that directory itself (and reads THORLABS_DLL_PATHS).
  • I fixed some comments that disagreed with the drivers. For example, the default soft-limit mode is "partial", not "disallow".

Behavior changes for the changelog

  • ppcl55x: errors are now TimeoutError / OSError (previously a magic value or a NameError).
  • TSL550 wavelength_logging: raises TimeoutError / OSError instead of RuntimeError.
  • Arduino: analog_read raises TimeoutError after 1 s by default, instead of hanging.

Testing

  • New hardware-free tests use fake serial ports and VISA managers:
    • ppcl55x: timeouts, bad checksum, chained read errors, recovery after a failure, no overlapping exchanges between threads, and on() with unanswered no-ops;
    • TSL550 wavelength logging;
    • Arduino analog_read;
    • RTO's ResourceManager lifetime.
  • Each regression test fails on master.
  • Full suite: 451 passed.
  • Still needs a hardware check: ppcl55x on()/off() and normal reads on a real PPCL550/551. PPCL550 autoconnect() cycles between success and failure #28 (the PPCL550 autoconnect flapping) may be related to the hang fixed here.
  • The examples could only be checked by compiling them and comparing every call against the driver source. They need Kinesis hardware to run.

Found but not fixed (KDC101)

I found these while checking the examples against the drivers:

  • stop_mode getter: compares an int to a c_short, so it is always False, and its elif checks jog_mode by mistake. It raises when jog mode is continuous, and kcube_jogging.py prints it.
  • jog_velocity setter: overwrites the jog acceleration with move_acceleration.
  • jog_acceleration setter: sends the move velocity, unconverted, instead of the jog velocity.
  • move_continuous: reads e.errcode on a RuntimeError, which has no such attribute.
  • close(): raises AttributeError on an instance that never connected.

🤖 Generated with Claude Code

sequoiap and others added 5 commits October 5, 2026 11:25
_receive raised CommunicationError, which was never imported, so a bad
checksum or failed read became a NameError (#88), and a timeout returned
the in-band value (0xFF, 0xFF, 0xFF, 0xFF) (#58). Now a timeout raises
TimeoutError and a failed read or bad checksum raises OSError, chained to
the serial error (#60). They are built-in exceptions because Pyro can only
rebuild those on a client, and a Pyro CommunicationError (the issue's
suggestion) would make the client's proxy drop its connection and lock.

_communicate's lock was created fresh on every call, so it serialized
nothing, and a failed exchange never left the ticket queue, so every later
call spun forever. A single per-laser lock replaces both. The wait for a
reply now sleeps instead of busy-polling.

on() and off() send ten no-ops as a delay; one going unanswered is still
tolerated, as it was when timeouts returned a value.

Fixes #88, fixes #58

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
They also caught KeyboardInterrupt and SystemExit, and threw away the
original error. TSL550.wavelength_logging's loop never repeated; it now
raises TimeoutError saying how far the data got, or OSError chained to the
serial error, instead of an opaque RuntimeError. The commented-out TSL550
test scaffold is replaced with real tests.

Fixes #60

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
analog_read spun at full CPU until the pin reported, forever if it never
did, hanging the whole daemon. It now takes a timeout (default 1 s),
sleeps between polls, and raises TimeoutError.

Fixes #57

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
connect() dropped its ResourceManager as soon as it returned; it is now
kept and closed by close(). The timeout docstring said the default was
1 ms; it is 1000 ms.

Fixes #79

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
They passed serial numbers to the constructor; instruments are constructed
without arguments and connected with connect(). The examples now close the
stage in a finally block, drop the Kinesis DLL path workaround (the
thorlabs_kinesis package adds that directory itself), and correct comments
that disagreed with the drivers.

Fixes #41

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sequoiap
sequoiap merged commit 99bb12e into master Oct 5, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ppcl55x raises CommunicationError without importing it, so read errors become NameError

1 participant