Asynchronous DmxInput #8

functionpointer · 2021-07-12T22:24:28Z

Add async DmxInput capability.

DmxInput is now running in the background,
using a DMA channel and the DMA_IRQ0 interrupt.

This approach gives the following advantages:

Supports both blocking and non-blocking use
CPU knows when a packet was received
Supports any number of channels (not just powers of 2)
Requires no memory alignment of the buffer
Minimal (one interrupt per dmx packet) cpu overhead
No periodic function calls from the main sketch

Current downsides:

~~Only one instance of DmxInput is supported. Could be changed if required.~~
~~Breaking change of the existing API. This is mostly due to the pre-allocated buffer. Could be changed at the expense of an extra "start" function call for the async mode.~~

Alternative implementations

An alternative method using the DMA ring feature was also
considered. This method would have meant almost no cpu load
with just one interrupt every 2**32 received channels.
However, this would require the number of channels
to be a power of two, the buffer would need to be naturally aligned
(256 channels would need a 256-byte aligned buffer), and there would be
no way to tell if dmx signals have stopped coming in.

DmxInput is now running in the background, using a DMA channel and the DMA_IRQ0 interrupt. This approach gives the following advantages: - Supports both blocking and non-blocking use - CPU knows when a packet was received - Supports any number of channels (not just powers of 2) - Requires no memory alignment of the buffer An alternative method using the DMA ring feature was also considered. This method would have meant almost no cpu load with just one interrupt every 2**32 received channels. However, this would require the number of channels to be a power of two, the buffer would need to be naturally aligned (256 channels would need a 256-byte aligned buffer), and there would be no way to tell if dmx signals have stopped coming in.

jostlowe · 2021-07-13T08:04:51Z

Hi!

This looks really cool! Nice work :) Unfortunately, I'm on vacation with all my Arduino stuff left at home. I'll take a look at the code and run some tests sometime next week. I'll look over the code and perhaps leave some comments in the meantime.

Could you take a look at the Arduino Linter checks? It seems to be complaining about the naming of the file RGB_input_async.ino

jostlowe · 2021-07-13T08:07:13Z

src/DmxInput.cpp

@@ -10,6 +10,13 @@
 #include <irq.h>
 #include <Arduino.h> // REMOVE ME

+int DmxInput::_buffer_size() {
+    int num_channels = _end_channel+1;
+    return num_channels+((4-num_channels%4)%4);


Is the purpose of this to 32-align the buffer in memory?

No.
This code rounds the buffer size up to the next whole integer. This is needed because the pio is configured to push 32 bits (four channels) at once. This makes the DMA transfers more efficient.

This approach has one downside: We may not be able to receive the last few (up to 3) DMX channels. However, i believe most DMX signals will contain a multiple of four channels so in most cases we can receive everything.

jostlowe · 2021-07-13T08:08:23Z

src/DmxInput.cpp

+
+   //initialize with some starting values
+    for(int i=0;i<_buffer_size();i++) {
+        ((volatile uint8_t*)_buf)[i] = i;


Would it be possible to use memset here instead?

Not with the increasing i, right?
I added this code mostly for debugging. For production there isn't actually a reason to initialize the memory at all.

jostlowe · 2021-07-13T08:14:44Z

src/DmxInput.cpp

@@ -116,4 +167,6 @@ void DmxInput::end()

    // Unclaim the sm
    pio_sm_unclaim(_pio, _sm);
+
+    free((void*)_buf);


Is the call to free necessary when the buffer isn't heap allocated?

No, good catch! I was experimenting with a malloc`d buffer and forgot to remove this. I will remove it.

jostlowe · 2021-07-13T08:30:32Z

I have 2 general questions about the architecture of the code:

How much of a hassle would it be to make the code not single-instance? I was hoping to make a multi-port DMX-Ethernet node implementation with the possibility for multiple inputs using this library :)
In your opinion: What do you think about making the async input into its own class? As of now, the user must poll the DMX Input to see if a new frame has arrived. While this is more resource consuming than your elegant IRQ sollution, it is more comparable to the "standard" way Arduino likes the format of their libraries, and is what a beginner user would expect. By making a separate class, we might also avoid a breaking API change.

functionpointer · 2021-07-14T22:00:21Z

How much of a hassle would it be to make the code not single-instance? I was hoping to make a multi-port DMX-Ethernet node implementation with the possibility for multiple inputs using this library :)

Shouldn't be much hassle. The interrupt handler would need to know all the instances and which one caused the interrupt. Currently i just have a global variable, hence only a single instance.

In your opinion: What do you think about making the async input into its own class? As of now, the user must poll the DMX Input to see if a new frame has arrived. While this is more resource consuming than your elegant IRQ sollution, it is more comparable to the "standard" way Arduino likes the format of their libraries, and is what a beginner user would expect. By making a separate class, we might also avoid a breaking API change.

A separate class would contain lots of duplicated code. I think the breaking API change can be removed without a separate class. It would make the async read a but more cumbersome to use, but that tradeoff might be worth it.

jostlowe · 2021-07-16T10:13:40Z

Nice work! You're an absolute legend!

I'll run some tests next week and merge the pull request and create a release if everything is in order :)

BollaBerg · 2021-08-21T16:10:22Z

LGTM 🚀

functionpointer added 2 commits July 13, 2021 00:10

Cleanup async DmxInput

64509d5

functionpointer mentioned this pull request Jul 12, 2021

Adding IRQ to indicate a full frame has been received #7

Closed

jostlowe reviewed Jul 13, 2021

View reviewed changes

functionpointer added 5 commits July 15, 2021 00:36

Remove breaking API change and add read_async()

4535e23

Rename async example to match folder name

7373a56

Support up to 8 DmxInput at once

2ebf43e

Fix DmxInput pio program loaded too many times

5c6e249

Fix DmxInput forced jumps

64a077a

jostlowe merged commit 4f0f76d into jostlowe:main Aug 21, 2021

kripton mentioned this pull request Dec 15, 2021

DmxInput: start_channel is not honored #15

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Asynchronous DmxInput #8

Asynchronous DmxInput #8

functionpointer commented Jul 12, 2021 •

edited

Loading

jostlowe commented Jul 13, 2021

jostlowe Jul 13, 2021

functionpointer Jul 14, 2021

jostlowe Jul 13, 2021

functionpointer Jul 14, 2021

jostlowe Jul 13, 2021

functionpointer Jul 14, 2021

jostlowe commented Jul 13, 2021

functionpointer commented Jul 14, 2021

jostlowe commented Jul 16, 2021

BollaBerg commented Aug 21, 2021

Asynchronous DmxInput #8

Asynchronous DmxInput #8

Conversation

functionpointer commented Jul 12, 2021 • edited Loading

jostlowe commented Jul 13, 2021

jostlowe Jul 13, 2021

Choose a reason for hiding this comment

functionpointer Jul 14, 2021

Choose a reason for hiding this comment

jostlowe Jul 13, 2021

Choose a reason for hiding this comment

functionpointer Jul 14, 2021

Choose a reason for hiding this comment

jostlowe Jul 13, 2021

Choose a reason for hiding this comment

functionpointer Jul 14, 2021

Choose a reason for hiding this comment

jostlowe commented Jul 13, 2021

functionpointer commented Jul 14, 2021

jostlowe commented Jul 16, 2021

BollaBerg commented Aug 21, 2021

functionpointer commented Jul 12, 2021 •

edited

Loading