spacemanspiff2007 / PyArtNet

"Python wrappers for the Art-Net protocol to send DMX over Ethernet"
GNU General Public License v3.0
67 stars 13 forks source link

Stop thread when not fading #22

Closed Breina closed 1 year ago

Breina commented 1 year ago

Stops the thread when it's not needed, freeing up resources.

Replaces #18

hynekcer commented 1 year ago

Please, have you ever tried the code?

You have a big bug here:

class DmxClient:
    def __init__(self, host: str, port: int):
        # a mangled attribute name that is different from an attribute "__host" in subclasses
        self.__host = host
        ...
    # never used "__host" in methods

class ArtNetClient(DmxClient):
    def __init__(self, host: str, ...):
        super().__init__(host, port)
        ... # never initialized __host

    def update(self, ...):
        self.__socket.sendto(... self.__host ...)  # AttributeError

AttributeError: 'ArtNetClient' object has no attribute '_ArtNetClient__host'. The attribute __host is a class-private member that is not inherited.

A poor "fix" by self._DmxClient__host would be possible, but it seems a sign of bad design. Consider that the original method "update()" is never called in unit tests where it is replaced by a stub. Examples in readme are also not tested and should work.

e.g. the example with channel.add_fade(...) must be replaced by await channel.add_fade(...)

I think that automatic start and stop of threads could cause a problem with the callback example in readme.

hynekcer commented 1 year ago

Though ArtDmx packet with opcode 0x5000 is the most important packet type in ArtNet to transfer DMX512 protocol, some DMX class should be a subclass or some ArtNet, not conversely how you reverted it against the original correct implementation.

See