lancaster-university / codal-nrf52

MIT License
3 stars 15 forks source link

Do you plan to separate NRF_I2S from NeoPixel? #7

Closed OpusK closed 4 years ago

OpusK commented 4 years ago

Hi,

Currently, I2S is directly dependent on NeoPixel (in sync_serial branch). Do you plan to separate from NeoPixel as "NRF52I2S"?

https://github.com/lancaster-university/codal-nrf52/blob/3362b483e106c338cec1b8a49d4e586402c8e143/source/NRF52Neopixel.cpp#L40

jamesadevine commented 4 years ago

Hi @OpusK,

I think we should but we have no reason to do so at the moment. I2S is convenient as it allows the hardware to asynchronously write neopixel bytes without processor intervention.

We'd be happy to see a contribution that provides an alternate implementation, though. Perhaps configured via a macro?

OpusK commented 4 years ago

I think it would be nice to create an NRF52I2S class like NRF52SPI and make it standalone, then use that class in Neopixel. (We have to use I2S in microphone and speaker)

And this is a question from curiosity. Is there a reason to include device-dependent code such as Neopixel in nrf52core? I thought codal-nrf52 would be a design that only implements code for the nrf52 core. (peripherals..)

jamesadevine commented 4 years ago

@OpusK I completely agree, when it is needed it will be written :smile: feel free to have a go yourself!

And this is a question from curiosity. Is there a reason to include device-dependent code such as Neopixel in nrf52core?

I'm not sure of the specifics in this case, but generally what tends to happen is that we have a generalised interface ("driver-models") in codal-core that are implemented in libraries. The generality of the driver dictates where it goes. In this case the neopixel driver implementation for the nrf52 is likely to remain the same regardless of the device instance (i.e. ble-nano, bluefruit). This approach essentially has zero cost as code will only be included in the final binary if it is used. Commonly device models (i.e. a device instance) include the drivers required by a device (see codal-ble-nano).

OpusK commented 4 years ago

Thanks for the detailed answer. As you say, there are files for a specific sensor in codal-core, so I know what codal-core is trying to do.