JnyJny / busylight

Control USB connected presence lights from multiple vendors via the command-line or web API.
Apache License 2.0
220 stars 25 forks source link

Kuando BusyLight does not release usb port #44

Closed henryruhs closed 3 years ago

henryruhs commented 3 years ago

Hey Erik,

I fully implemented your lib to Chroma Feedback today. It seems that Kuando BusyLight has an issue to release the usb port... for my application I do this little hack to catch exceptions on startup.

https://github.com/redaxmedia/chroma-feedback/blob/master/chroma_feedback/consumer/kuando_busylight/api.py#L24

Here is a test-script, it works fine with the with Blink1 and BlinkStick - just uncomment one of them to give it try

#!/usr/bin/env python3

from time import sleep

from busylight.lights.kuando import BusyLight as API
#from busylight.lights.thingm import Blink1 as API
#from busylight.lights.agile_innovations import BlinkStick as API

first_light = API.first_light()
first_light.on((0, 255, 255))
print(first_light.name)
sleep(1)

# this seems not to release the busylight
first_light.release()
sleep(1)

# lights are [] for some reason - properly the usb port is blocked
lights = API.all_lights()
print(lights)

for light in lights:
    print(light.name)
    light.on((255, 0, 0))
    sleep(1)
    light.off()

Thanks for your time :-)

JnyJny commented 3 years ago

That is an unfortunate side-effect of how the Kuando Busylight is implemented. If the device doesn't receive a keep-alive write within 30 seconds of the previous write, it will turn itself off. Contrast that with the Embrava Blynclight which will continue whatever it was previously commanded until interrupted, unplugged from the host or the machine is powered off.

I wanted all the lights to operate in a similar manner: if you turn them on, they stay on until turned off. Therefore, the Kuando Busylight (and others) have threads which run the animation indefinitely. This indefinite animation also holds the device open until the current process exits, which means the status of the light cannot be modified from another process.

It turns out, I anticipated this problem when I wrote busylight.manger.LightManager. I suggest accessing the lights via the LightManager instead of instantiating them directly.

henryruhs commented 3 years ago

Thanks for the explanation, I studied the code and figured out the keepalive thread. This was the reason I used the release() method as this should cancel the thread and therefore release the device again!?

def release(self) -> None:
    with self.lock:
        try:
            self.keepalive_thread.cancel()
            del self._keepalive_thread
        except:
            pass
        super().release()

As mentioned, I use this just to approve connected devices - is there something we can do to check this another way or properly introduce another helper function for this usecase that does not open a thread?

try
    api.first_light().release()
except (USBLightIOError, USBLightNotFound):
    exit(wording.get('connection_no').format('KUANDO BUSYLIGHT') + wording.get('exclamation_mark'))

What is different using the LightManager ? I did not have the time to give it a try but should this work with my hacky solution?

JnyJny commented 3 years ago

I ran your scripts since what you were reporting shouldn't have occurred and sure enough, there is (was) a bug in the threading code that runs the keep-alive thread which affects only the Kuando BusyLight. Version 0.12.2 ( 944b4a0c81fbefd5055a639d474aacc932d9860e) fixes the threading bug and addresses breakage in the web API. Calling methods on Kunado BusyLight instantiations should no longer block indefinitely.

henryruhs commented 3 years ago

Thanks for the new release - it works way better but still has a tiny bug.

Just in case the device has not been read and the thread properly not been opened, the release() method goes into an infinite loop:

#!/usr/bin/env python3

from busylight.lights.kuando import BusyLight as API

first_light = API.first_light()

# uncomment one of the next lines and it works
#first_light.off()
#print(first_light.name)
first_light.release()

lights = API.all_lights()
print(lights)

for light in lights:
    print(light.name)
    light.on((255, 0, 0))
JnyJny commented 3 years ago

Good news, it's not hung. The keepalive generator for the Kuando BusyLight only has 15 seconds to send a keep alive packet to the hardware. I could send a keep alive every 1 second which would be 15x number of I/Os actually required to keep the device alive. Sending a keep alive every 15 seconds runs the risk of drift due to process scheduling which could result in a keep alive packet arriving late at the hardware. I chose to send a packet every 7.5 seconds. So the thread wakes up, sends the keep alive packet and goes to sleep for 7.5 seconds. While the thread is in time.sleep() it is unresponsive. I've tested this with:

from busylight.lights.kuando import BusyLight

from contextlib import contextmanager
import time

@contextmanager
def timer(label: str) -> None:
    t0 = time.time()
    yield
    t1 = time.time()
    print(f"{label:>11} dt: {t1-t0:4.2f} seconds")

with timer("first_light"):
    l = BusyLight.first_light()

with timer("on"):
    l.on((0, 255, 0))

with timer("release"):
    l.release()

with timer("acquire"):
    l.acquire(True)

with timer("off"):
    l.off()

Running this script with a Kuando BusyLight attached results in:

$ python3 ./tst.py 
first_light dt: 0.01 seconds
         on dt: 0.00 seconds
    release dt: 7.51 seconds
    acquire dt: 0.01 seconds
        off dt: 0.00 seconds

As predicted, the release method returns in ~7.5 seconds, waiting for the keep alive thread to wake up and recognize that it has been cancelled. The trade off is running the keep alive loop fast in order to be responsive to release requests (which typically happen when an application is exiting) or running it slow to reduce the amount of unnecessary I/O to the device.

There's some accounting I can do in the keep alive generator function which could reduce the latency for responding to the cancel while still not flooding the device with unhelpful keep alive messages.

henryruhs commented 3 years ago

Interesting, can you bring this conclusion into a new release that makes the call API.first_light().release() work? You script works for me but mine still hangs forever - it is not just the 15 seconds that is defined here self.keepalive_thread.join(15)

Why not make the timeout adjustable for the user? I have no API in mind but that could make my life properly easier as that first "are you there" call is just for initial testing and the "normal usage" call could have a way better balanced timeout?

JnyJny commented 3 years ago

I've just pushed (ca44b2f1e368fcd94a20fa8426e82ae92096ee55). Now the test code from the previous comment reports:

$ python3 ./tst.py
first_light dt: 0.01 seconds
         on dt: 0.00 seconds
    release dt: 0.10 seconds
    acquire dt: 0.00 seconds
        off dt: 0.00 seconds

At the expense of more busy-waiting, the latency of the release method has been reduced to 0.1 seconds.

Your test script results in:

$ python3 foo.py
[BusyLight(vendor_id=0x27bb, product_id=0x3bcd, path=b'IOService:/AppleACPIPlatformExpert/PCI0@0/AppleACPIPCI/TRP0@7/IOPP/PXSX@0/IOPP/pci-bridge@0/IOPP/pci1b73,1100@0/AppleUSBXHCIFL1100@00000000/AppleUSB20XHCIPort@00100000/USB2.1 Hub@00100000/AppleUSB20Hub@00100000/AppleUSB20HubPort@00110000/USB2.1 Hub@00110000/AppleUSB20Hub@00110000/AppleUSB20HubPort@00114000/BUSYLIGHT@00114000/IOUSBHostInterface@0/AppleUserUSBHostHIDDevice', ...)]
Kuando Busylight
$
henryruhs commented 3 years ago

It works perfect now and is super responsive... thank you so much for that fix and that insane release cycles.