ola
ola copied to clipboard
RFC: Multi-universe support for usbdmx-based devices
Hi, this is not planned to be merged (at least not in the current form ;)). It's mainly in relation to #1712 so a proper review of this code can be made and we can get the best solution from @DanielG and the changes here.
These changes modify all usbdmx (= libusb) based devices to allow multiple output ports instead of just one. Therefore, the parameter portId is passed through all functions related to sending a DmxBuffer.
Missing in this PR is mainly the documentation adaption and testing (since many places in the code assume single-universe devices).
It also adds new USB VIDs and PIDs used by "Frank Sievertsen's FX5" dongle that I don't have to test. I was just using the same IDs with my RP2040-based proof of concept since it implements the same USB interface (the one also used by the Nodle U1).
@kripton thanks for posting these patches for discussion! Let me know if I got this right: Your approach is basically to plumb through an integer portId to every function that might need to case on it even if currently it's just ignored. Since the generic Port classes already have PortId you re-use this to avoid having to do more plumbing there, particularly around GenericOutputPort::WriteDMX.
My approach on the other hand basically amounts to creating custom Port subclasses that take ownership of dedicated UsbTranciever intsances. The reason I opted to doing it this way is because my custom device supports multiple USB interfaces and as such it's desirable to have multiple queues for the packets going into or coming out of the USB host controller anyway. Since the UsbTranciever classes always come with a dedicated thread that works out quite nicely.
There is one nasty bit of hackery in my patches currently and that's that I just make MyWidget::SendDMX crash since that doesn't give me the universe/portId I need. The reason that's OK to do is IIRC because only the GenericPort classes use this method and since I'm shipping my own MyPorts it doesn't matter. The method is demanded by the current SimpleWidget interface though so I can't just not implement it unfortunately.
In my downstream patches I try to not touch the ola core as much as I can so they can be rebased across releases more easily so I didn't actually try this, but I do think we should be able to refactor this problem away.
Anyway, so this Port-owns-Transciever is what's good for my device, but I do see how in general having multiple queues/threads wouldn't necessarily be desirable so I wonder how we could unify those two approaches into one that works for both our use-cases.