Added DMXCreator 512 Basic USB interface #1136

FloEdelmann · 2016-10-14T13:47:27Z

I reverse-engineered the USB protocol using WireShark and USBPcap. This is my first try, so there may be some functions that I didn't implement or that could be better, please just let me know.

DMXCreator sends two URB packets for each change:

A constant byte string to endpoint 1 that indicates new data.
The actual DMX data to endpoint 2.

~~It currently only supports 256 channels, I may work on that later.~~ (Fixed in df44d5a)
Also, there is a bug that lets it consume way too much memory in synchronous libusb mode until it eventually gets killed.

see http://www.dmx512.ch/512.html I reverse-engineered the USB protocol using WireShark and USBPcap. This is my first try, so there may be some functions that I didn't implement or that could be better, please just let me know. DMXCreator sends two URB packets for each change: 1. A constant byte string to endpoint 1 that indicates new data. 2. The actual DMX data to endpoint 2. It currently only supports 256 channels, I may work on that later. Also, there is a bug that lets it consume way too much memory in synchronous libusb mode until it eventually gets killed.

FloEdelmann · 2016-10-14T14:01:11Z

There is one more bug: It doesn't handle unplugging correctly so the next time it gets plugged in, it refuses to connect because of the missing serial number (there can always only be one DMXCreator device at a time).

…dmxcreator

debian/ola.udev

plugins/usbdmx/AsyncPluginImpl.cpp

peternewman

A few comments so far.

We're also about to merge #1134 which will need a bit of minor tidying on your side to fix.

plugins/usbdmx/DMXCreatorFactory.cpp

peternewman · 2016-10-17T17:54:25Z

plugins/usbdmx/UsbDmxPlugin.cpp

@@ -56,7 +56,7 @@ bool UsbDmxPlugin::StartHook() {
  }

  unsigned int debug_level;
-  if (!StringToInt(m_preferences->GetValue(LIBUSB_DEBUG_LEVEL_KEY) ,
+  if (!StringToInt(m_preferences->GetValue(LIBUSB_DEBUG_LEVEL_KEY),


plugins/usbdmx/UsbDmxPlugin.cpp

peternewman · 2016-10-17T18:01:14Z

I think the unplugging bug will be because m_missing_serial_number never gets reset when the serial number less widget gets unplugged. I suspect this bug affects all the widgets with the same code.

peternewman · 2016-10-17T18:03:36Z

I'd imagine you send a different constant to endpoint 1 to send the other half of the universe, or perhaps just a second packet to endpoint 2 before you send the endpoint 1 data (i.e. sending to endpoint 1 may reset the slot counter back to 0).

peternewman · 2016-10-17T18:04:08Z

Hopefully the synchronous issue can be fixed, if not, it probably needs to be disabled for the moment.

peternewman · 2016-10-18T02:54:27Z

The other PR is in, so you can resync this.

* allow all 512 channels to be sent * disabled synchronous mode for now * made timeout much shorter * changed display name

FloEdelmann · 2016-10-18T22:14:59Z

It was exactly as you said: Just send two packets after another to endpoint 2. But I also had to change the constant that is sent to endpoint 1. It took me quite a while to figure that out 🙈

The unplugging bug and the synchronous mode issue unfortunately do both still exist. I'll look into them soon.

peternewman · 2016-10-19T01:20:54Z

Please can you add the device to the list in plugins/usbdmx/UsbDmxPlugin.h too.

plugins/usbdmx/AsyncUsbSender.cpp

plugins/usbdmx/DMXCreator.cpp

peternewman · 2016-10-19T01:54:27Z

plugins/usbdmx/DMXCreator.cpp

+  }
+
+  bool PerformTransfer(const DmxBuffer &buffer) {
+    // if we only wanted to send the first half, the last byte would be 0x01


I take it you've not worked out what the other bytes in the packet do?

Unfortunately not. I just have this one device for testing, I guess the other DMXCreator dongles use other bytes.

peternewman · 2016-10-19T02:04:06Z

plugins/usbdmx/DMXCreatorFactory.cpp

+  OLA_INFO << "Found a new DMXCreator device";
+  // Unfortunately, DMXCreator devices don't provide any additional information
+  // that identify them. So we have to stick with just testing vendor and
+  // product ids. Also, since DMXCreator devices don't have serial numbers and


Do you know for certain that none have serial numbers?

The m_adaptor->GetDeviceInfo(usb_device, descriptor, &info) call ought to succeed if the device is present, even if it returns no other info, so you could do the following still, and just remove the vendor/device name checks:

LibUsbAdaptor::DeviceInformation info; if (!m_adaptor->GetDeviceInfo(usb_device, descriptor, &info)) { return false; }

Ok, I'll do that. I'm not completely sure that none of those devices have serial numbers but I think they don't since they do not even return valid manufacturer / product names.

Thanks. As you say, I suspect they're unlikely to have serial numbers, but it would be a shame to be missing the functionality if they did (given it's already been written).

plugins/usbdmx/DMXCreatorFactory.cpp

plugins/usbdmx/DMXCreatorFactory.h

plugins/usbdmx/UsbDmxPlugin.cpp

peternewman · 2016-10-19T02:14:30Z

Can you also add the 512 Basic bit to the filenames too please, given they already have other interfaces we might want to support in future.

* rename "DMXCreator" and "DMXCreator 512 Basic" to "DMXCreator512Basic" to be consistent * add serial check again * more minor fixes

causing doxygen to fail

peternewman

A few more minor comments.

AUTHORS

debian/ola.udev

plugins/usbdmx/AsyncPluginImpl.cpp

plugins/usbdmx/DMXCreator512Basic.cpp

plugins/usbdmx/DMXCreator512BasicFactory.cpp

plugins/usbdmx/DMXCreator512BasicFactory.h

plugins/usbdmx/SyncPluginImpl.cpp

plugins/usbdmx/UsbDmxPlugin.cpp

FloEdelmann · 2016-10-20T11:48:20Z

Everything should be fine now 😉

peternewman · 2016-10-21T15:51:03Z

plugins/usbdmx/DMXCreator512Basic.cpp

+    return true;
+  }
+
+  m_dmx_buffer = new DmxBuffer(buffer);  // force copy


You're copying the buffer just to see if it's changed?

Is there a better way?

It was partly just a comment that you could use it for the buffer.Get/GetRange calls.

I honestly don't see an advantage in that.

Not that I can immediately think of.

No specific advantage, but it would just mean you sort of take control of that data and only use that data in the plugin. You've currently got an odd mix of the locally cached and freshly fed in data.

Actually, there is an improvement, you can switch to m_dmx_buffer = buffer; Due to some magic we do in the background, it will copy the data if it needs to, but otherwise just leave it as a pointer.

FloEdelmann · 2016-10-21T16:55:44Z

Travis failed due to the GitHub outage, I guess.

https://status.github.com/

plugins/usbdmx/DMXCreator512Basic.cpp

nomis52 · 2016-10-24T15:51:22Z

plugins/usbdmx/DMXCreator512Basic.cpp

+  }
+
+  bytes_sent = 0;
+  r = m_adaptor->BulkTransfer(handle, ENDPOINT_2,


I'd move this code into a function since it's repeated 3 times.

plugins/usbdmx/DMXCreator512Basic.cpp

…dmxcreator

FloEdelmann · 2016-10-25T12:11:17Z

Sorry, I pushed by accident. Just ignore this commit until I comment again.

FloEdelmann · 2016-10-25T17:43:09Z

Now the BulkTransfer works as before. The size was wrong.
Also, I reduced the wait time to 50ns, that still works as expected.

FloEdelmann · 2016-10-26T08:16:00Z

Travis' PyChecker failed again due to network outage. Don't know if it was Sourceforge or Travis itself. At least it was not me ;)

Anyway, that test has nothing to do with my changes and the compiler tests come out well.

FloEdelmann · 2016-11-01T21:17:12Z

Any updates on this? I believe I have addressed all of your requested changes @peternewman @nomis52

peternewman · 2016-11-02T14:07:11Z

Hi @FloEdelmann , sorry I've been rather busy, I'll try and look in the next day or two.

plugins/usbdmx/DMXCreator512Basic.cpp

peternewman

Just a few more changes please.

plugins/usbdmx/DMXCreator512Basic.cpp

peternewman · 2016-11-08T16:14:13Z

@nomis52 any final comments from you?

FloEdelmann · 2016-11-09T21:17:25Z

Yay, it's done! 🎉

peternewman · 2016-11-09T22:13:39Z

Yep thanks for all your hard work @FloEdelmann , now onto the next one...

FloEdelmann added 3 commits October 14, 2016 15:32

update name in LICENSE comment

95259bb

Merge branch 'master' into usb-dmxcreator

3011473

FloEdelmann added 2 commits October 14, 2016 16:40

Lint issues, move udev to right place

327944c

Merge branch 'usb-dmxcreator' of github.com:FloEdelmann/ola into usb-…

6b7e077

…dmxcreator

peternewman self-assigned this Oct 17, 2016

peternewman reviewed Oct 17, 2016

View reviewed changes

debian/ola.udev Show resolved Hide resolved

peternewman reviewed Oct 17, 2016

View reviewed changes

plugins/usbdmx/AsyncPluginImpl.cpp Show resolved Hide resolved

peternewman requested changes Oct 17, 2016

View reviewed changes

peternewman added this to the 0.11.0 milestone Oct 17, 2016

FloEdelmann mentioned this pull request Oct 18, 2016

Added udev rule for DMXCreator USB interface OpenLightingProject/raspberrypi#9

Merged

FloEdelmann added 2 commits October 18, 2016 19:35

merge upstream/master

5e02d85

several fixes

df44d5a

* allow all 512 channels to be sent * disabled synchronous mode for now * made timeout much shorter * changed display name

peternewman requested changes Oct 19, 2016

View reviewed changes

plugins/usbdmx/AsyncUsbSender.cpp Show resolved Hide resolved

peternewman requested changes Oct 19, 2016

View reviewed changes

FloEdelmann changed the title ~~Added DMXCreator USB interface~~ Added DMXCreator 512 Basic USB interface Oct 19, 2016

peternewman and others added 3 commits October 19, 2016 11:19

Fix some minor spacing nits

0fd30e9

Fixed requested changes

6295fde

* rename "DMXCreator" and "DMXCreator 512 Basic" to "DMXCreator512Basic" to be consistent * add serial check again * more minor fixes

Fix missing @param in doc comment

c20cbbf

causing doxygen to fail

peternewman requested changes Oct 20, 2016

View reviewed changes

Fix spaces in user-visible name

d3d0c4a

peternewman reviewed Oct 21, 2016

View reviewed changes

peternewman and others added 4 commits October 22, 2016 15:11

Merge branch 'master' into usb-dmxcreator

27e53a8

Merge branch 'master' into usb-dmxcreator

affa91b

change buffer assignment

ccbbab3

Merge branch 'master' into usb-dmxcreator

4bfa68b

nomis52 requested changes Oct 24, 2016

View reviewed changes

peternewman and others added 3 commits October 25, 2016 01:00

Merge branch 'master' into usb-dmxcreator

d3393cf

fix leaking, memset and function

557bbc1

Merge branch 'usb-dmxcreator' of github.com:FloEdelmann/ola into usb-…

9597d14

…dmxcreator

reduce wait time, fix function call

8912801

Merge branch 'master' into usb-dmxcreator

886c297

peternewman reviewed Nov 8, 2016

View reviewed changes

plugins/usbdmx/DMXCreator512Basic.cpp Show resolved Hide resolved

peternewman requested changes Nov 8, 2016

View reviewed changes

fixed comment and spaces

a2c77a4

peternewman approved these changes Nov 8, 2016

View reviewed changes

nomis52 approved these changes Nov 9, 2016

View reviewed changes

Merge branch 'master' into usb-dmxcreator

8509a7d

peternewman merged commit 8c0aea9 into OpenLightingProject:master Nov 9, 2016

FloEdelmann deleted the usb-dmxcreator branch November 9, 2016 21:17

FloEdelmann mentioned this pull request May 3, 2018

Feature/de interface #1410

Merged

FloEdelmann mentioned this pull request Jul 4, 2019

Support for Eurolite MK2 with conflict checks #1556

Merged

Added DMXCreator 512 Basic USB interface #1136

Added DMXCreator 512 Basic USB interface #1136

Conversation

FloEdelmann commented Oct 14, 2016 • edited

FloEdelmann commented Oct 14, 2016

peternewman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peternewman commented Oct 17, 2016

peternewman commented Oct 17, 2016

peternewman commented Oct 17, 2016

peternewman commented Oct 18, 2016

FloEdelmann commented Oct 18, 2016 • edited

peternewman commented Oct 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FloEdelmann Oct 19, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peternewman commented Oct 19, 2016

peternewman left a comment

Choose a reason for hiding this comment

FloEdelmann commented Oct 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FloEdelmann commented Oct 21, 2016

Choose a reason for hiding this comment

FloEdelmann commented Oct 25, 2016

FloEdelmann commented Oct 25, 2016 • edited

FloEdelmann commented Oct 26, 2016 • edited

FloEdelmann commented Nov 1, 2016

peternewman commented Nov 2, 2016

peternewman left a comment

Choose a reason for hiding this comment

peternewman commented Nov 8, 2016

FloEdelmann commented Nov 9, 2016

peternewman commented Nov 9, 2016

FloEdelmann commented Oct 14, 2016 •

edited

FloEdelmann commented Oct 18, 2016 •

edited

FloEdelmann Oct 19, 2016 •

edited

FloEdelmann commented Oct 25, 2016 •

edited

FloEdelmann commented Oct 26, 2016 •

edited