DmxInput: start_channel is not honored

[kripton]

DmxInput::begin has a parameter called start_channel. It's meant to ignore the first X channels and only read input values from that value on.
However, the value is not actually used as expected. It is however (incorrectly) used to calculate the buffer size here:

Pico-DMX/src/DmxInput.cpp

Line 163 in e493ea2

DMXINPUT_BUFFER_SIZE(_start_channel, _num_channels)/4, // transfer count,

I wouldn't know a proper solution to tell the PIO state machine OR the DMA to ignore the first X bytes. Of course, we could let the DMX write into an intermediary buffer and then copy only the requested channels to the buffer provided by the library user. But in the worst case, that would mean copying 511 byte in an IRQ handler. Not what I would call ideal.

As such I would propose to remove the start_channel functionality

[kripton]

It looks like the start_channel feature was in and working (only when using read) before #8, but was removed when read was re-written to use read_async to do its job:
https://github.com/jostlowe/Pico-DMX/pull/8/files#diff-42a9a8ae12afd6562fa620becac01e3c263d29fd4cbcd832a35abbca6cd76fa6L84

[Gavin-Perry]

I can't get DMX channel one to be read properly from my Chauvet Light board.
IF I use DMXOutput and DMXInput then it works "right?". Or are both off by one?
How is start byte (normally 0) handled? Does the returned buffer put channel 1 at index 0 or 1?
I think it would be clear if the entire DMX packet were read in including the Start byte so that channel 1 is indexed as [1]
This would let one use RDM as well as avoiding confusion between index of array and channel number.

Is the problem here?
in .cpp we have &_pio->rxf[_sm], // src
DMXINPUT_BUFFER_SIZE(_start_channel, _num_channels), // transfer count,
(is it src, dest, count?)
the .h file defines
#define DMXINPUT_BUFFER_SIZE (start_channel, num_channels) (num_channels+1)
in the version I have, though it is different in the one referenced by kripton in the previous comment
#define DMXINPUT_BUFFER_SIZE(start_channel, num_channels) ((start_channel+num_channels+1)+((4-(start_channel+num_channels+1)%4)%4))
Presumably this was before the 32 bit transfer not doing 513 bytes issue was resolved by doing 8 bit transfers

[kripton]

How is start byte (normally 0) handled? Does the returned buffer put channel 1 at index 0 or 1? I think it would be clear if the entire DMX packet were read in including the Start byte so that channel 1 is indexed as [1] This would let one use RDM as well as avoiding confusion between index of array and channel number.

I didn't try it out lately but according to the Readme any my memory, the 0th byte in the buffer is the start code, the byte indexed 1 is the value of the 1st channel in the universe

[Gavin-Perry]

Yes, that is how it is supposed to work according to DMX standard. My question is how the buffer array is done in the library since it isn't working right.

…

-- Gavin Perry, PhD Chief Technology Officer Meridian Electric Company 2392 Grissom Drive St Louis MO 63146 http://meridianlighting.com cell: 314 406-0697 <http://voice.google.com/calls?a=nc,%2B13144060697>

On Sun, Mar 27, 2022 at 1:29 PM kripton ***@***.***> wrote: How is start byte (normally 0) handled? Does the returned buffer put channel 1 at index 0 or 1? I think it would be clear if the entire DMX packet were read in including the Start byte so that channel 1 is indexed as [1] This would let one use RDM as well as avoiding confusion between index of array and channel number. I didn't try it out lately but according to the Readme any my memory, the 0th byte in the buffer is the start code, the byte indexed 1 is the value of the 1st channel in the universe — Reply to this email directly, view it on GitHub <#15 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AVR4PPZZIKTXL45JY3ILJCDVCCZILANCNFSM5KCB4KXQ> . You are receiving this because you commented.Message ID: ***@***.***>

[kripton]

If it's not working right (= as expected), I would assume that the DMX sender sends "short frames" with fewer than 513 slots.This is perfectly fine according to the standard but the library currently cannot cope with it.
As you said, it would be helpful to have an IRQ on BREAK or MAB but it'S currently just not implemented.

[Gavin-Perry]

Thanks for your help. As it turns out the pins I used for setting the address (GPIO 26-28) Don't work with digitalRead() or gpio_get() Minimum code gpio_set_dir(26, GPIO_IN); gpio_pull_up(26); gpio_set_dir(27, GPIO_IN); gpio_pull_up(27); gpio_set_dir(28, GPIO_IN); gpio_pull_up(28); Serial.println(gpio_get(26)); Serial.println(gpio_get(27)); Serial.println(gpio_get(28)); Always return 0! So I wasn't setting the address I thought! Work around was to use analogRead() and set a threshold value (400) below = 0; above =1. Don't know if the problem is arduino-pico but I suspect not, since gpio_get is reaching through via the SDK and not using arduino code.

…

-- Gavin Perry, PhD Chief Technology Officer Meridian Electric Company 2392 Grissom Drive St Louis MO 63146 http://meridianlighting.com cell: 314 406-0697 <http://voice.google.com/calls?a=nc,%2B13144060697>

On Sat, Apr 16, 2022 at 2:56 PM kripton ***@***.***> wrote: If it's not working right (= as expected), I would assume that the DMX sender sends "short frames" with fewer than 513 slots.This is perfectly fine according to the standard but the library currently cannot cope with it. As you said, it would be helpful to have an IRQ on BREAK or MAB but it'S currently just not implemented. — Reply to this email directly, view it on GitHub <#15 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AVR4PP4RJAIDQOKVMI3YXMTVFMLQPANCNFSM5KCB4KXQ> . You are receiving this because you commented.Message ID: ***@***.***>

[queekusme]

@Gavin-Perry i've used 26-28 on my Pi Pico (non arduino) without any issues so I presume that the issues you're referencing are arduino specific

I haven't tried the ADC specifically however if adc_init is called from somewhere in the arduino source it may block the default GPIO functions... I had this issue with I2C

I can't confirm this just yet as I'm currently afk for a few days but suspect it may be the case

I also don't have an arduino pico setup so can't confirm that way however from what I do know, this is most likely the reason...

[Gavin-Perry]

Thanks for the idea. I don't use adc_init, and I explicitly did call gpio_set_dir(i,GPIO_OUT) and then gpio_pull {up or down}
The pull worked for setting my defaults.
I'd call gpio_set_function() but the choices don't include simple gpio input as far as I can tell.
All the fancy uses are documented, not the simplest which should be (and claimed to be) default.

[queekusme]

@Gavin-Perry you shouldn't need to set the function,

The enum is defined here (https://raspberrypi.github.io/pico-sdk-doxygen/group__hardware__gpio.html) as gpio_function,

If you're just reading digital inputs then you can ignore function, that's for connecting the hardware spi/i2c/pwm modules to the pins, heck you shouldn't even need to call that for adc, as that's handled by adc_init

[Gavin-Perry]

I now realize that now. Just flailing at options to figure out the problem with digital input which should be the default. And learning some of the cool things that this baby can do.

[Gavin-Perry]

Looks like I need to use gpio_init() because the Arduino boot loader isn't enabling them.

[MarcusGarfunkel]

I'd like to mention that this issue is affecting my use of this library.

[jostlowe]

I'm a bit short on time these days! @kripton would it be possible for you to author a pull request to remove/fix the feature?

[kripton]

Unfortunately, I currently also don't have much spare time to donate to this project. So please don't expect a PR for this, soon.

However, I don't see why this is a big blocker: As long as there are "full packets" = start code + 512 channels on the wire, the reception should be fine. Yes, you will need to provide a 513 byte large buffer, receive the complete packet and then use only the parts you are interested in. In the worst case, it's a 512 byte large memcpy/memmove operation.

@MarcusGarfunkel: Any point I missed? Is that workaround not working for you?

[MarcusGarfunkel]

@kripton, @jostlowe Hi, thanks for the response. The work around is fine, I just believe most people assume the behavior to work as expected, and it results in unexpected behavior. I appreciate the effort.