Add a Eurolite serial number check

[aroffringa]

This MR adds an option 'eurolite_serials' to the usb dmx plugin. This option accepts a comma separated list of serial numbers that are assumed to be Eurolite mk2 devices (as long as their product and vendor ids also match).

This allows using the Eurolite MK2 without enabling the 'enable_eurolite_mk2' option. The advantage is that is avoids the conflicts that 'enable_eurolite_mk2' produces with other plugins. These
conflicts make it impossible to connect the mk2 simultaneously together with most other interfaces to have multiple universes and/or input universes.

I have tested this by connecting an Enttec pro together with the Eurolite Mk2, and this works.

I still need to update documentation -- happy to finish that too if this approach is considered acceptable.

[peternewman]

Thanks @aroffringa .

Yeah I think this is an acceptable compromise, its just a bit niche to have bothered doing it before unless you're in the position of having both (the benefits of open source).

I think there's just the one main suggested change, and then just look at the logging and readme but you've covered most of that to make it nice and easy,

It'll want a little cotton wool to ensure they don't add blank lines etc.

[aroffringa]

@peternewman I've adopted your comments and added the config key to the usbdmx readme.

[peternewman]

Some minor wording tweaks (and a possible extension idea).

It might be good to set a default (blank) serial key here:

ola/plugins/usbdmx/UsbDmxPlugin.cpp

Lines 95 to 115 in 96aa95e

	bool UsbDmxPlugin::SetDefaultPreferences() {
	if (!m_preferences) {
	return false;
	}
	bool save = m_preferences->SetDefaultValue(
	LIBUSB_DEBUG_LEVEL_KEY,
	UIntValidator(LIBUSB_DEFAULT_DEBUG_LEVEL, LIBUSB_MAX_DEBUG_LEVEL),
	LIBUSB_DEFAULT_DEBUG_LEVEL);
	save |= m_preferences->SetDefaultValue(
	EuroliteProFactory::ENABLE_EUROLITE_MK2_KEY,
	BoolValidator(),
	false);
	if (save) {
	m_preferences->Save();
	}
	return true;
	}

Although it would then throw the empty line error each time...

I don't know what you think about this (maybe as a future enhancement for another PR):

1. Start up OLAd in enable_eurolite_mk2
2. If no (non-blank) eurolite_mk2_serial set (at startup), it adds each found one to the list
3. So workflow wise you could plug in your Eurolite dongles, turn on the setting, run up OLA. Turn off the setting and plug in your other devices.
4. Upsides you wouldn't have to read logs or find serial numbers via other methods
5. Downsides some potential confusion if you don't follow the right workflow
6. Alternative suggestion is do the same thing but put them in another key (e.g. discovered_eurolite_mk2_serial) which you could rename if required.

FYI you can merge the suggestions straight in via the web UI if you want to:
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/incorporating-feedback-in-your-pull-request#applying-suggested-changes

[aroffringa]

I've added the blank default and made sure that if a single blank serial number is given no warning is produced. I'm not sure I understand your suggestion, do you want ola to rewrite the config file and add the serial numbers? As a user that's really not what I would expect ola to do, to be honest, but I might be misunderstanding you.

[aroffringa]

@peternewman is this MR ready to merge?

[peternewman]

Sorry for the big delay in getting back to you! This wasn't raised from an issue was it @aroffringa that we ought to link it to?

Quite a bit of this is just a few minor style things, feel free to just merge my suggestions as is for them. Please can you also resync with master.

Just to check, when you specify by serial and have one or more of FTDI, Stageprofi or USB Pro enabled, does it all play nicely, or does ConflictsWith need tweaking still? USB Pro would be fine, as there's two way comms and the Eurolite won't behave as a USB Pro (plus you said you've tested it), I can't remember about Stageprofi (so I probably need to check the code), but won't FTDI just happily start spewing DMX data at it when it finds it, which would just make it a race condition which plugin detects it first?

[peternewman]

I've added the blank default and made sure that if a single blank serial number is given no warning is produced.

Great thanks!

I'm not sure I understand your suggestion, do you want ola to rewrite the config file and add the serial numbers? As a user that's really not what I would expect ola to do, to be honest, but I might be misunderstanding you.

I was just trying to think through how someone will use it. If like you I've got one Eurolite Mk2 and one Enttec, I find I can't use them both together unless I enable the serial number mode, but how do I find the serial number? If what's printed on the bottom of the unit is exactly what I need to type in, then perfect, but otherwise I was thinking OLA could effectively pull either any serials it finds, or the serials it found in the last run, into the config file, for you to copy and paste. Otherwise you need to either use lsusb or trawl through all the OLA logs or similar. But you're the one using the kit, does that make sense as a workflow or seem redundant? It can definitely be a future PR/open issue and certainly isn't a blocker to getting this merged!

[aroffringa]

I was just trying to think through how someone will use it. If like you I've got one Eurolite Mk2 and one Enttec, I find I can't use them both together unless I enable the serial number mode, but how do I find the serial number? If what's printed on the bottom of the unit is exactly what I need to type in, then perfect, but otherwise I was thinking OLA could effectively pull either any serials it finds, or the serials it found in the last run, into the config file, for you to copy and paste. Otherwise you need to either use lsusb or trawl through all the OLA logs or similar. But you're the one using the kit, does that make sense as a workflow or seem redundant? It can definitely be a future PR/open issue and certainly isn't a blocker to getting this merged!

Someone would now indeed have to look at the OLA output or lsusb to find the serial number -- this is indeed not super user friendly. What would be really useful is if this could be configured on the web interface (including that the interface would show the serial numbers), but that's a whole different scope ;). What's currently implemented at least demonstrates the use of the serial number to solve the conflict. In fact, given that quite some modules conflict with each other, a future idea could even be to incorporate this in a more generic way so that the user would be shown the possible serial ids and select the right device for it.

For now I wouldn't want to enlarge the scope of this pull request — as it is it seems to work.

I again applied your suggestions and updated the branch to master, so I think it is ready to be merged.

[peternewman]

LGTM thanks for all the hard work @aroffringa and sorry it's taken me quite so long to get this reviewed and in!

[peternewman]

I think outstanding tasks are as follows:

For you to test:

Just to check, when you specify by serial and have one or more of FTDI, Stageprofi or USB Pro enabled, does it all play nicely, or does ConflictsWith need tweaking still? USB Pro would be fine, as there's two way comms and the Eurolite won't behave as a USB Pro (plus you said you've tested it), I can't remember about Stageprofi (so I probably need to check the code), but won't FTDI just happily start spewing DMX data at it when it finds it, which would just make it a race condition which plugin detects it first?

For me to do and then for you to test: #1932

We need to fix the code a few lines below here, where it then ignores our special serial number and uses the device and bus number, for the MK2 at least, as that should mean the patch persists even when you plug it into a different USB port, which it won't do currently.

You should be able to see that either way as the serial number should show in the device name/description in OLA.

I've open a new issue to track potential simplification for the user of finding serial numbers to put in the config - #1929

[aroffringa]

For you to test:

Just to check, when you specify by serial and have one or more of FTDI, Stageprofi or USB Pro enabled, does it all play nicely, or does ConflictsWith need tweaking still? USB Pro would be fine, as there's two way comms and the Eurolite won't behave as a USB Pro (plus you said you've tested it), I can't remember about Stageprofi (so I probably need to check the code), but won't FTDI just happily start spewing DMX data at it when it finds it, which would just make it a race condition which plugin detects it first?

@peternewman I enabled the stageprofi, opendmx and ftdi plugins one by one and checked if the Eurolite continues to work. The serial plugin (that's what you mean with usb pro enabled?) I have indeed already tested extensively.

Conclusion: the Stageprofi and opendmx plugins are not an issue, but enabling the FTDI device causes the Eurolite to no longer work. It's somewhat weird because the Eurolite is still detected and patched, but it seems that the FTDI device also tries to claim it and that causes issues. This is the log:

plugins/usbdmx/EuroliteProFactory.cpp:124: Found a probable new Eurolite USB-DMX512-PRO MK2 device with matching serial AK069T9T
plugins/usbdmx/EurolitePro.cpp:100: Using interface 0
olad/plugin_api/DeviceManager.cpp:105: Installed device: EurolitePro USB Device:12-eurolite-AK069T9T
olad/plugin_api/PortManager.cpp:119: Patched 12-eurolite-AK069T9T-O-0 to universe 1
plugins/usbdmx/AsyncPluginImpl.cpp:304: Device 2:13 claimed by EuroliteProFactory
libs/usb/HotplugAgent.cpp:172: USB hotplug event: 2:1 @0x5567b92f2400 [add]
libs/usb/LibUsbThread.cpp:50: -- Starting libusb thread
common/thread/Thread.cpp:200: Thread , policy SCHED_OTHER, priority 0
libs/usb/LibUsbThread.cpp:35: ----libusb event thread is running
olad/PluginManager.cpp:200: Started USB
olad/PluginManager.cpp:195: Trying to start FTDI USB DMX
plugins/ftdidmx/FtdiWidget.cpp:133: Found 1 FTDI devices with PID: 24577.
plugins/ftdidmx/FtdiWidget.cpp:189: Found FTDI device. Vendor: 'FTDI', Name: 'FT232R USB UART', Serial: 'AK069T9T'
plugins/ftdidmx/FtdiWidget.cpp:133: Found 0 FTDI devices with PID: 24593.
plugins/ftdidmx/FtdiDmxDevice.cpp:60: Widget FT232R USB UART has 1 interfaces.
plugins/ftdidmx/FtdiWidget.cpp:230: Setting interface to: 1
common/thread/Thread.cpp:200: Thread , policy SCHED_OTHER, priority 0
plugins/ftdidmx/FtdiDmxDevice.cpp:75: Successfully added 1/1 interfaces.
olad/plugin_api/DeviceManager.cpp:105: Installed device: FT232R USB UART with serial number : AK069T9T :13-AK069T9T
olad/PluginManager.cpp:200: Started FTDI USB DMX
...

When I then try to use the interface, I get lots of these errors:

plugins/usbdmx/AsyncUsbTransceiverBase.cpp:118: libusb_submit_transfer returned LIBUSB_ERROR_IO

[peternewman]

@peternewman I enabled the stageprofi, opendmx and ftdi plugins one by one and checked if the Eurolite continues to work. The serial plugin (that's what you mean with usb pro enabled?) I have indeed already tested extensively.

Yes sorry, serial=usbpro.

Conclusion: the Stageprofi and opendmx plugins are not an issue, but enabling the FTDI device causes the Eurolite to no longer work. It's somewhat weird because the Eurolite is still detected and patched, but it seems that the FTDI device also tries to claim it and that causes issues.

Yeah as I feared, they discover in different ways (via libusb and libftdi) so I don't think they can share the file locking stuff, so it just becomes a free for all, and the FTDI plugin will just pump DMX packets at any DMX interface it finds. Opendmx works similar, but needs a kernel driver so is more niche and I think if the device got detected as that it wouldn't be found via libusb and vice versa so we're probably okay there (but it may be safe to disable it too)...

I assume the StageProfi plugin did detect but then ignored the device? Looking at the code, there is some two way comms to establish it really is a StageProfi so I think we're safe there.

Anyway would you mind doing another minor PR please that makes it so that only StageProfi and serial/usbpro can avoid conflicting when the specific serial setting is on (i.e. ensure the FTDI and OpenDMX continue to conflict and they can't break as your test just found)?

So some other lines around here:
https://github.com/aroffringa/ola/blob/ee8796219dd55a50cbbb6360a9804797fbc0b07d/plugins/usbdmx/UsbDmxPlugin.cpp#L122-L129