cereal2nd / velbus-aio

Velbus Asyncio
Apache License 2.0
15 stars 12 forks source link

Add (minimal) support for VBMDALI #48

Closed niobos closed 2 years ago

niobos commented 2 years ago

This PR is an attempt to add (minimal) support for the VBMDALI module. This is currently limited to LedModule DALI devices, since these are the only ones I have access to.

Relates to #32

niobos commented 2 years ago

Thank you for the approval so far. That way I'm confident that I'm not going in a direction you don't want.

I may be rebasing and reordering these commits, though. I'm not sure what will happen to your preliminary approval.

The factory is so that I can do something like this:

class Module:
    @classmethod
    def factory(cls, module_type) -> Module:
        if module_type == "VMBDALI":
            return VmbDali(module_type)
        # else:
        return Module(module_type)
    ...
    def some_method(self):
        ...  # default behaviour

class VmbDali(Module):
    """Behaves exactly like Module, but allows module-specific overrides"""
    ...
    def some_method(self):
        ...  # different behaviour vs Module
cereal2nd commented 2 years ago

this commit is becoming so big it becomes dangerous to break things as we have no testing for the velbusaio lib on its own it hink we may need to merge this one in smaller chunks certainly the "general" areas that are touched

niobos commented 2 years ago

I tried to keep each individual commit very contained, but indeed, the whole PR is getting big... What parts would you like to merge first?

cereal2nd commented 2 years ago

for example, first all the messaging changes, then the controller/module changes

this will give us the possibility to test this before we add another change on top. I just want to be sure we do not destabalize the system, if we had automated test for the core system this would be less of a concern.

Maybe its time to start adding testcases ...

niobos commented 2 years ago

I've rebased this PR on top of the new master (after #49). It now only contains module/channel changes, separated in 3 commits for easier reviewing.

niobos commented 2 years ago

Thank you for reviewing this and accepting this PR!