Skip to content

Allow setting list of pins to sample after construction #38

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

Merged
merged 4 commits into from
Apr 13, 2023

Conversation

tfry-git
Copy link
Contributor

@tfry-git tfry-git commented Apr 5, 2023

Arguably, in user code, the list of pins to sample will typically be known and fixed at compile-time.

However, when wrapping this into a library (Mozzi; for the purpose of providing a cross-platform analog read mechanism), it will be very helpful to have a way to adjust the pins to sample after construction.

tfry-git added 2 commits April 5, 2023 21:43
Arguably, in user code, the list of pins to sample will typically be known and fixed at compile-time.

However, when wrapping this into a library (Mozzi; for the purpose of providing a cross-platform analog read mechanism), it will be very helpful to have a way to adjust the pins to sample after construction.
@github-actions
Copy link

github-actions bot commented Apr 5, 2023

Memory usage change @ 04f1275

Board flash % RAM for global variables %
arduino:mbed_giga:giga 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
Click for full report table
Board examples/Advanced/ADC_Multi
flash
% examples/Advanced/ADC_Multi
RAM for global variables
% examples/Advanced/ADC_Multi_Channel
flash
% examples/Advanced/ADC_Multi_Channel
RAM for global variables
% examples/Advanced/ADC_Multi_To_DAC
flash
% examples/Advanced/ADC_Multi_To_DAC
RAM for global variables
% examples/Advanced/ADC_Serial_Plotter
flash
% examples/Advanced/ADC_Serial_Plotter
RAM for global variables
% examples/Advanced/ADC_To_DAC
flash
% examples/Advanced/ADC_To_DAC
RAM for global variables
% examples/Advanced/DAC_One_Channel
flash
% examples/Advanced/DAC_One_Channel
RAM for global variables
% examples/Advanced/DAC_Sine_wave
flash
% examples/Advanced/DAC_Sine_wave
RAM for global variables
% examples/Advanced/DAC_Two_Channels
flash
% examples/Advanced/DAC_Two_Channels
RAM for global variables
% examples/Beginner/Audio_Playback
flash
% examples/Beginner/Audio_Playback
RAM for global variables
% examples/Beginner/Waveform_Generator
flash
% examples/Beginner/Waveform_Generator
RAM for global variables
%
arduino:mbed_giga:giga 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0
Click for full report CSV
Board,examples/Advanced/ADC_Multi<br>flash,%,examples/Advanced/ADC_Multi<br>RAM for global variables,%,examples/Advanced/ADC_Multi_Channel<br>flash,%,examples/Advanced/ADC_Multi_Channel<br>RAM for global variables,%,examples/Advanced/ADC_Multi_To_DAC<br>flash,%,examples/Advanced/ADC_Multi_To_DAC<br>RAM for global variables,%,examples/Advanced/ADC_Serial_Plotter<br>flash,%,examples/Advanced/ADC_Serial_Plotter<br>RAM for global variables,%,examples/Advanced/ADC_To_DAC<br>flash,%,examples/Advanced/ADC_To_DAC<br>RAM for global variables,%,examples/Advanced/DAC_One_Channel<br>flash,%,examples/Advanced/DAC_One_Channel<br>RAM for global variables,%,examples/Advanced/DAC_Sine_wave<br>flash,%,examples/Advanced/DAC_Sine_wave<br>RAM for global variables,%,examples/Advanced/DAC_Two_Channels<br>flash,%,examples/Advanced/DAC_Two_Channels<br>RAM for global variables,%,examples/Beginner/Audio_Playback<br>flash,%,examples/Beginner/Audio_Playback<br>RAM for global variables,%,examples/Beginner/Waveform_Generator<br>flash,%,examples/Beginner/Waveform_Generator<br>RAM for global variables,%
arduino:mbed_giga:giga,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0

I shall test my PR before submitting...
@github-actions
Copy link

github-actions bot commented Apr 5, 2023

Memory usage change @ 82ae24a

Board flash % RAM for global variables %
arduino:mbed_giga:giga 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
Click for full report table
Board examples/Advanced/ADC_Multi
flash
% examples/Advanced/ADC_Multi
RAM for global variables
% examples/Advanced/ADC_Multi_Channel
flash
% examples/Advanced/ADC_Multi_Channel
RAM for global variables
% examples/Advanced/ADC_Multi_To_DAC
flash
% examples/Advanced/ADC_Multi_To_DAC
RAM for global variables
% examples/Advanced/ADC_Serial_Plotter
flash
% examples/Advanced/ADC_Serial_Plotter
RAM for global variables
% examples/Advanced/ADC_To_DAC
flash
% examples/Advanced/ADC_To_DAC
RAM for global variables
% examples/Advanced/DAC_One_Channel
flash
% examples/Advanced/DAC_One_Channel
RAM for global variables
% examples/Advanced/DAC_Sine_wave
flash
% examples/Advanced/DAC_Sine_wave
RAM for global variables
% examples/Advanced/DAC_Two_Channels
flash
% examples/Advanced/DAC_Two_Channels
RAM for global variables
% examples/Beginner/Audio_Playback
flash
% examples/Beginner/Audio_Playback
RAM for global variables
% examples/Beginner/Waveform_Generator
flash
% examples/Beginner/Waveform_Generator
RAM for global variables
%
arduino:mbed_giga:giga 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0
Click for full report CSV
Board,examples/Advanced/ADC_Multi<br>flash,%,examples/Advanced/ADC_Multi<br>RAM for global variables,%,examples/Advanced/ADC_Multi_Channel<br>flash,%,examples/Advanced/ADC_Multi_Channel<br>RAM for global variables,%,examples/Advanced/ADC_Multi_To_DAC<br>flash,%,examples/Advanced/ADC_Multi_To_DAC<br>RAM for global variables,%,examples/Advanced/ADC_Serial_Plotter<br>flash,%,examples/Advanced/ADC_Serial_Plotter<br>RAM for global variables,%,examples/Advanced/ADC_To_DAC<br>flash,%,examples/Advanced/ADC_To_DAC<br>RAM for global variables,%,examples/Advanced/DAC_One_Channel<br>flash,%,examples/Advanced/DAC_One_Channel<br>RAM for global variables,%,examples/Advanced/DAC_Sine_wave<br>flash,%,examples/Advanced/DAC_Sine_wave<br>RAM for global variables,%,examples/Advanced/DAC_Two_Channels<br>flash,%,examples/Advanced/DAC_Two_Channels<br>RAM for global variables,%,examples/Beginner/Audio_Playback<br>flash,%,examples/Beginner/Audio_Playback<br>RAM for global variables,%,examples/Beginner/Waveform_Generator<br>flash,%,examples/Beginner/Waveform_Generator<br>RAM for global variables,%
arduino:mbed_giga:giga,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0

@aentinger
Copy link
Contributor

Hi @tfry-git ☕ 👋

Can you please provide an example show-casing how you'd use this feature?

@tfry-git
Copy link
Contributor Author

Can you please provide an example show-casing how you'd use this feature?

For clarification: Do you want an example of how I would actually use this (in order to judge, whether the proposed change makes sense), or do you want me to provide a user visible example for inclusion in examples-dir?

I'll happily provide the second, if that's what you mean.

As for the first: My actual use-case is a library for audio synthesis (Mozzi). Note that Mozzi is cross platform, and the user-facing API already exists, so from the Mozzi point of view this PR is about enabling an implementation on the Giga. More specifically, we have a function uint16_t mozziAnalogRead(uint8_t pin). This behaves similar to analogRead(), but with the difference that it does not wait blocking for a conversion to happen, but rather returns a cached value (and ensures that an actual reading will be taken as soon as possible). The rationale is, that in our context a blocking delay is typically prohibitive (buffer underrun), but readings are going to be taken repeatedly, anyway.

Regarding the API, none of the other existing platform ports require prior specification of the pins that will be used in mozziAnalogRead(), it's fully implicit. We want the same existing code base to work on the Giga, unchanged. I.e. the semantics are roughly:

mozziAnalogRead() -> update list of pins to sample -> trigger hardware-specific non-blocking readings of these pins -> cache the resulting values -> to be returned in a subsequent call of mozziAnalogRead()

Regarding possible way to accomplish this, I see the following options:

  • copy the implementation code, modified to our needs -> clearly not a good idea
  • dynamically allocate / de-allocate appropriate AdvancedADC instances at runtime -> possible, but more cumbersome on our end, and will hide factual global RAM usage
  • this PR
  • an alternative option would be to have n_channels and adc_pins as protected rather than private members -> keeps the API more restricted, but would allow us to add what we need inside Mozzi

@aentinger
Copy link
Contributor

Hi @tfry-git ☕ 👋

Judging by the current state of your PR I consider it non-invasive, hence would be happy to merge.

For clarification: Do you want an example of how I would actually use this (in order to judge, whether the proposed change makes sense), or do you want me to provide a user visible example for inclusion in examples-dir?

I'll happily provide the second, if that's what you mean.

Yes, this would be about the second option. If you could add such an example to this PR I'll be happy to merge 🙇 .

@CLAassistant
Copy link

CLAassistant commented Apr 12, 2023

CLA assistant check
All committers have signed the CLA.

@tfry-git
Copy link
Contributor Author

Example added.

@github-actions
Copy link

Memory usage change @ 4c36569

Board flash % RAM for global variables %
arduino:mbed_giga:giga 0 - 0 0.0 - 0.0 0 - 0 0.0 - 0.0
Click for full report table
Board examples/Advanced/ADC_Multi
flash
% examples/Advanced/ADC_Multi
RAM for global variables
% examples/Advanced/ADC_Multi_Channel
flash
% examples/Advanced/ADC_Multi_Channel
RAM for global variables
% examples/Advanced/ADC_Multi_Channel_Dynamic
flash
% examples/Advanced/ADC_Multi_Channel_Dynamic
RAM for global variables
% examples/Advanced/ADC_Multi_To_DAC
flash
% examples/Advanced/ADC_Multi_To_DAC
RAM for global variables
% examples/Advanced/ADC_Serial_Plotter
flash
% examples/Advanced/ADC_Serial_Plotter
RAM for global variables
% examples/Advanced/ADC_To_DAC
flash
% examples/Advanced/ADC_To_DAC
RAM for global variables
% examples/Advanced/DAC_One_Channel
flash
% examples/Advanced/DAC_One_Channel
RAM for global variables
% examples/Advanced/DAC_Sine_wave
flash
% examples/Advanced/DAC_Sine_wave
RAM for global variables
% examples/Advanced/DAC_Two_Channels
flash
% examples/Advanced/DAC_Two_Channels
RAM for global variables
% examples/Beginner/Audio_Playback
flash
% examples/Beginner/Audio_Playback
RAM for global variables
% examples/Beginner/Waveform_Generator
flash
% examples/Beginner/Waveform_Generator
RAM for global variables
%
arduino:mbed_giga:giga 0 0.0 0 0.0 0 0.0 0 0.0 N/A N/A N/A N/A 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0 0 0.0
Click for full report CSV
Board,examples/Advanced/ADC_Multi<br>flash,%,examples/Advanced/ADC_Multi<br>RAM for global variables,%,examples/Advanced/ADC_Multi_Channel<br>flash,%,examples/Advanced/ADC_Multi_Channel<br>RAM for global variables,%,examples/Advanced/ADC_Multi_Channel_Dynamic<br>flash,%,examples/Advanced/ADC_Multi_Channel_Dynamic<br>RAM for global variables,%,examples/Advanced/ADC_Multi_To_DAC<br>flash,%,examples/Advanced/ADC_Multi_To_DAC<br>RAM for global variables,%,examples/Advanced/ADC_Serial_Plotter<br>flash,%,examples/Advanced/ADC_Serial_Plotter<br>RAM for global variables,%,examples/Advanced/ADC_To_DAC<br>flash,%,examples/Advanced/ADC_To_DAC<br>RAM for global variables,%,examples/Advanced/DAC_One_Channel<br>flash,%,examples/Advanced/DAC_One_Channel<br>RAM for global variables,%,examples/Advanced/DAC_Sine_wave<br>flash,%,examples/Advanced/DAC_Sine_wave<br>RAM for global variables,%,examples/Advanced/DAC_Two_Channels<br>flash,%,examples/Advanced/DAC_Two_Channels<br>RAM for global variables,%,examples/Beginner/Audio_Playback<br>flash,%,examples/Beginner/Audio_Playback<br>RAM for global variables,%,examples/Beginner/Waveform_Generator<br>flash,%,examples/Beginner/Waveform_Generator<br>RAM for global variables,%
arduino:mbed_giga:giga,0,0.0,0,0.0,0,0.0,0,0.0,N/A,N/A,N/A,N/A,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0,0,0.0

Copy link
Contributor

@aentinger aentinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Thank you @tfry-git 🚀

@aentinger aentinger merged commit d989bcf into arduino-libraries:main Apr 13, 2023
@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants