Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Device doesn't unmute when the unmute request is sent to broadcast uid #10

Closed
rbarreiros opened this issue Apr 9, 2018 · 6 comments · Fixed by #13
Closed

Device doesn't unmute when the unmute request is sent to broadcast uid #10

rbarreiros opened this issue Apr 9, 2018 · 6 comments · Fixed by #13

Comments

@rbarreiros
Copy link

As the title states, it doesn't unmute when the request is made to broadcast as it only checks if packet is to self, should check also if it's to all.

I won't push a patch as the fix is simple enough.

DmxSerial2.cpp, line 525 should be:
if (packetIsForMe || packetIsForAll) {

Best regards,

@peternewman
Copy link
Contributor

It's actually going to want to be
if (packetIsForMe || packetIsForGroup || packetIsForAll) {

Likewise line 541 for the mute.

How did you find out @rbarreiros ; I see OLA doesn't currently have tests for this:
https://github.com/OpenLightingProject/ola/blob/master/tools/rdm/TestDefinitions.py

@rbarreiros
Copy link
Author

Hey Peter,
Hehehe, forgot the packetIsForGroup also. I thought the mute would respond to self only ?
I found it while using an arduino loaded with the responder example of DmxSerial2 as a test for an artnet controller with RDM support, initially with an STM32F103 (blue pill) but it's getting hard to stick everything in 64k flash and 20k ram....

@peternewman
Copy link
Contributor

Yeah I wasn't sure either, but the standard says:

7.6.3 Discovery Mute Message (DISC_MUTE)
A responder port shall set its Mute flag when it receives this message containing its UID, or a broadcast address.

Which is about as clear as you get for the RDM spec 😄 , I'm taking plural of broadcast address to include vendorcast too.

@peternewman
Copy link
Contributor

It is actually a bit more subtle, as we only want to respond to the unicast, but action any of uni, vendor or broadcast.
PR here, but needs testing:
#13

@rbarreiros do you still have your artnet controller?

@rbarreiros
Copy link
Author

Hey Peter, yes I do, but it's development is in standby, festival season, currently on small vacation, hope to get back at it late september, will test as soon as possible.

@peternewman
Copy link
Contributor

That's cool, I'm busy for a bit too. I'm hoping to do some testing on an Arduino base too, and ideally sort some OLA tests too.

peternewman referenced this issue Oct 5, 2018
Untested fix to ensure we act on vendorcast/broadcast mute/unmute
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 a pull request may close this issue.

2 participants