jostlowe / Pico-DMX

A library for inputting and outputting the DMX512-A lighting control protocol from a Raspberry Pi Pico
BSD 3-Clause "New" or "Revised" License
187 stars 21 forks source link

Asynchronous DmxInput #8

Closed functionpointer closed 3 years ago

functionpointer commented 3 years ago

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:

Current downsides:

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.

jostlowe commented 3 years ago

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 commented 3 years ago

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

  1. 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 :)
  2. 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 commented 3 years ago
  1. 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 commented 3 years ago

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 commented 3 years ago

LGTM 🚀