Skip to content

Adds support for AVLdiy.cn D512 usb dmx device, closes #1073 - #1134

Merged
peternewman merged 2 commits into
OpenLightingProject:masterfrom
thgjerde:master
Oct 18, 2016
Merged

Adds support for AVLdiy.cn D512 usb dmx device, closes #1073#1134
peternewman merged 2 commits into
OpenLightingProject:masterfrom
thgjerde:master

Conversation

@thgjerde

@thgjerde thgjerde commented Oct 3, 2016

Copy link
Copy Markdown

The AVLdiy.cn D512 uses the same protocol as the Anyma uDMX device.

@peternewman peternewman self-assigned this Oct 4, 2016
@peternewman peternewman added this to the 0.11.0 milestone Oct 4, 2016

@peternewman peternewman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some lint issues for starters.

I'm wondering if you can sensibly subclass some of the Anyma uDMX classes, or if it's worth it.

Comment thread plugins/usbdmx/AVLdiyD512.h Outdated
* @param serial the serial number of the widget.
*/
AVLdiyD512(ola::usb::LibUsbAdaptor *adaptor,
libusb_device *usb_device,

@peternewman peternewman Oct 4, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indent by one to match the (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread plugins/usbdmx/AVLdiyD512.h Outdated
*/
AVLdiyD512(ola::usb::LibUsbAdaptor *adaptor,
libusb_device *usb_device,
const std::string &serial)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indent by one to match the (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread plugins/usbdmx/AVLdiyD512.h Outdated
* @param serial the serial number of the widget.
*/
SynchronousAVLdiyD512(ola::usb::LibUsbAdaptor *adaptor,
libusb_device *usb_device,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indent by one to match the (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread plugins/usbdmx/AVLdiyD512.h Outdated
*/
SynchronousAVLdiyD512(ola::usb::LibUsbAdaptor *adaptor,
libusb_device *usb_device,
const std::string &serial);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indent by one to match the (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread plugins/usbdmx/AVLdiyD512.h Outdated
* @param serial the serial number of the widget.
*/
AsynchronousAVLdiyD512(ola::usb::LibUsbAdaptor *adaptor,
libusb_device *usb_device,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indent by one to match the (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread plugins/usbdmx/AVLdiyD512.h Outdated
*/
AsynchronousAVLdiyD512(ola::usb::LibUsbAdaptor *adaptor,
libusb_device *usb_device,
const std::string &serial);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indent by one to match the (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread debian/ola.udev
# udev rules for the anyma dmx device
SUBSYSTEM=="usb|usb_device", ACTION=="add", ATTRS{idVendor}=="16c0", ATTRS{idProduct}=="05dc", GROUP="plugdev"

# udev rules for the avldiy dmx device

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

Comment thread plugins/usbdmx/AVLdiyD512.cpp Outdated
*
* AVLdiyD512.cpp
* The synchronous and asynchronous AVLdiy D512 widgets.
* Copyright (C) 2014 Simon Newton

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct this to your name and 2016.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread plugins/usbdmx/AVLdiyD512.h Outdated
*
* AVLdiyD512.h
* The synchronous and asynchronous AVLdiy D512 widgets.
* Copyright (C) 2014 Simon Newton

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct this to your name and 2016.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread plugins/usbdmx/AVLdiyD512Factory.cpp Outdated
*
* AVLdiyD512Factory.cpp
* The factory for AVLdiy D512 widgets.
* Copyright (C) 2014 Simon Newton

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct this to your name and 2016.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread plugins/usbdmx/AVLdiyD512Factory.h Outdated
*
* AVLdiyD512Factory.h
* The factory for AVLdiy D512 widgets.
* Copyright (C) 2014 Simon Newton

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct this to your name and 2016.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@peternewman

Copy link
Copy Markdown
Member

If you resync @thgjerde the Mac tests should now pass.

@peternewman peternewman changed the title Adds support for AVLdiy.cn D512 usb dmx device Adds support for AVLdiy.cn D512 usb dmx device, closes #1073 Oct 7, 2016
@peternewman

Copy link
Copy Markdown
Member

This will close #1073 .

The AVLdiy.cn D512 uses the same protocol as the Anyma uDMX device.
@thgjerde

thgjerde commented Oct 8, 2016

Copy link
Copy Markdown
Author

I don't see an easy way to inherit from the AnymauDMX classes without refactoring all of the SimpleWidget inheritance tree.

@peternewman

Copy link
Copy Markdown
Member

Okay, I'd not really looked, just throwing it out there.

Can you avoid squashing in future please, as it makes my job harder to see if you've fixed the comments.

@peternewman

Copy link
Copy Markdown
Member

Can you also add a mention of your interface to: https://github.com/OpenLightingProject/ola/blob/master/plugins/usbdmx/UsbDmxPlugin.cpp#L95

You may also want to add yourself here:
https://github.com/OpenLightingProject/ola/blob/master/AUTHORS

@peternewman peternewman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few final bits from me.

Comment thread plugins/usbdmx/AVLdiyD512.cpp Outdated
};

bool AVLdiyThreadedSender::TransmitBuffer(libusb_device_handle *handle,
const DmxBuffer &buffer) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indent by one to match the (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread plugins/usbdmx/SyncPluginImpl.cpp Outdated
return true;
}

bool SyncPluginImpl::NewWidget(AVLdiyD512 *widget) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you put this below the AnymauDMX method.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread plugins/usbdmx/SyncPluginImpl.cpp Outdated
m_debug_level(debug_level),
m_preferences(preferences),
m_context(NULL) {
m_widget_factories.push_back(new AVLdiyD512Factory(&m_usb_adaptor));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you put this below the AnymauDMX method.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@peternewman

Copy link
Copy Markdown
Member

Aside from my final comments, anything else from yourself @nomis52 ?

@thgjerde

Copy link
Copy Markdown
Author

I think I have addressed all the requested changes.

@thgjerde

Copy link
Copy Markdown
Author

Please let me know if there is anything more I should fix.

@peternewman

Copy link
Copy Markdown
Member

Thanks @thgjerde , sorry I've been away, I'll take a proper look tonight and should be able to merge them from the looks of things.

@peternewman

Copy link
Copy Markdown
Member

LGTM

@peternewman
peternewman merged commit 35fda2d into OpenLightingProject:master Oct 18, 2016
peternewman referenced this pull request in nip3o/open-lighting Jun 7, 2018
@peternewman peternewman mentioned this pull request Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants