JnyJny/busylight

Error handling using new Exceptions

Closed this issue · 4 comments

This is more of a question rather than a bug report. At this point I don't have real devices with me, so please forgive me asking something I could figure out in couple of months - I just like to prepare the migration to your latest lib release.

In the current implementation of Chroma Feedback I use this to detect lights from not being connected.

from busylight.lights import USBLightIOError, USBLightNotFound
from busylight.lights.agile_innovative import BlinkStick as api

try:
	api.first_light().release()
except (USBLightIOError, USBLightNotFound):
	logger.error(wording.get('connection_not_found').format('agile_innovative_blinkstick'))
	sys.exit()

I saw you changed the exception naming and introduced a new method?

from busylight.lights import USBLightIOError, USBLightNotFound
from busylight.lights.agile_innovative import BlinkStick as api

try:
	api.first_light().is_pluggedin()
except (LightUnavailable, LightNotFound):
	logger.error(wording.get('connection_not_found').format('agile_innovative_blinkstick'))
	sys.exit()

Can I use this approach across all devices?

Yes, the property method is_pluggedin is designed to work across all Light subclasses. If you find an instance where it doesn't, that's a bug. The first_light class method will raise NoLightsFound and not LightUnavailable. The exception LightUnavailable only occurs when a light has been released via the release method and you are trying to reacquire exclusive access via the acquire method.

Thanks for the quick response. I still wonder if release() over is_pluggedin() is the better option to insure a connected and accessible / non busy device? Does the one send an operation and the other just return a properly outdated state?

According to the exceptions: I would expect a singular exception naming for first_light() and plural exception naming for all_lights() but that's not the case as all_lights() does not raise any exception. After quick research I figured out that LightNotFound are part of the API only. It's a bit confusing to me... :)

Since Light.all_lights() returns a list it follows that an empty list signifies no lights were found and no reason to raise an exception. The caller will always get the expected object instance returned to them, a list, but it will not always be populated with Light subclasses. The Light.first_light() class method only returns a Light subclass or raises an exception if no lights were found. I suppose I chose the plural NoLightsFound since I know the implementation will check for the existence of multiple different kinds of lights before giving up (depending of course where in the Light subclass hierarchy first_light is called from).

WRT to is_plugged, it is an abstract property method that is implemented in the HIDLight and SerialLight subclasses since the method for determining whether a light is available varies between to the two different I/O strategies. I don't understand why release would be favored since it only performs a close on the underlying serial or HID device. In most cases, users of Light subclasses should never need to use acquire and release methods. On some operating systems, for instance HID devices on MacOS, if the device is unplugged it is possible that it will not be assigned the same OS-specific path when plugged back in. Plug/unplug events should generally result in disposing of the current Light subclass instance and getting a new one when the light is available again.

Creating a light with exclusive=True will result in the underlying HID or serial device being acquired, IO performed for the requested operation, and then released. This approach introduces latency between initiating an operation and the operation completing, but it allows the lights to be accessed via another process which may be desired in some cases. That design point is the driver behind the acquire and release methods in this implementation.

Wow, thanks for that deep dive into the HID universe... I will close this issue for now.