Allow for sample rate to be changed when used as a library

[jgartrel]

This pull request allows for the ability to re-define key parameters within AMY without actually modifying the core AMY code. This allows AMY to be adapted for many platforms and use cases when used as a library. Resolves issue #551

[bwhitman]

I agree with this in spirit. An issue we had is the arduino environment seems to never allow anyone to #define things before a library is included. Are you using AMY in arduino and if so does this work for you ?

[jgartrel]

I was using it as included under the src folder. I intended to test it in the library context next.

[jgartrel]

Also, The limited code and changes provided in this pull request are provided freely and completely to the shorepine/amy project to be used as its maintainers see fit and under any copyright they deem approprate.

[jgartrel]

@bwhitman , You are right ... I tried to include it this way and it does NOT work, let me do some research and see if there is a way to accomplish this in a sustainable way.

#define AMY_SAMPLE_RATE 31250
#include <AMY-Arduino.h>

[jgartrel]

@bwhitman , Setting the define through the build.extra.flags mechanism (through a boards.local.txt or a platform.local.txt file) does seem to work, allowing for a pre define to change the sample rate.

I guess that leaves 3 options:

1. Just abandon the feature request Redefinable sample rate when used as a library #551
2. Expect that it may be consumed by this board-level or core-level definition mechanism, and call it done.
3. Keep looking for better ways to expose AMY_SAMPLE_RATE in a way that can be optimized by the compiler, yet still allow for it to be a configurable parameter.

Ultimately, I think this pull request provides some individuals a way to consume the AMY code as a read-only library without increasing the difficulty of maintaining AMY long-term. If we do find a better way, this is not complex to remove and would actually be removed in the natural workflow of such a change.

I vote to go forward with the pull request (but I am biased after all, given that I created the feature request) :-D

What do you all think?

[jgartrel]

This was tested by extending the definition of build.extra.flags as a part of the board definition file (boards.txt and boards.local.txt):

my_new_board.build.extra_flags=-DNRF52840_XXAA -D__BUILD_EPOCH__={extra.time.utc} -DAMY_SAMPLE_RATE=31250 -DAMY_ISSUE_425 -DSERIAL_BUFFER_SIZE=1024 ...

Ultimately this gets translated into a compile command through platform.txt (and platform_local.txt):

# Compile patterns
# ----------------

## Compile c files
recipe.c.o.pattern="{compiler.path}{compiler.c.cmd}" {compiler.c.flags} -DF_CPU={build.f_cpu} -DARDUINO={runtime.ide.version} -DARDUINO_{build.board} -DARDUINO_ARCH_{build.arch} '-DARDUINO_BSP_VERSION="{version}"' {compiler.c.extra_flags} {build.extra_flags} {build.flags.nrf} {includes} "{source_file}" -o "{object_file}"

## Compile c++ files 
recipe.cpp.o.pattern="{compiler.path}{compiler.cpp.cmd}" {compiler.cpp.flags} -DF_CPU={build.f_cpu} -DARDUINO={runtime.ide.version} -DARDUINO_{build.board} -DARDUINO_ARCH_{build.arch} '-DARDUINO_BSP_VERSION="{version}"' {compiler.cpp.extra_flags} {build.extra_flags} {build.flags.nrf} {includes} "{source_file}" -o "{object_file}" 

## Compile S files
recipe.S.o.pattern="{compiler.path}{compiler.S.cmd}" {compiler.S.flags} -DF_CPU={build.f_cpu} -DARDUINO={runtime.ide.version} -DARDUINO_{build.board} -DARDUINO_ARCH_{build.arch} {compiler.S.extra_flags} {build.extra_flags} {build.flags.nrf} {includes} "{source_file}" -o "{object_file}"

This pattern seem to be used by other libraries, especially those bundled with various arduino cores to pass configurable defines into the library.

[bwhitman]

I like (2) to be honest. But i'm still hunting for a good (3) in Arduino. We have definitely tested making sample rate, channels etc an amy_config runtime arg, and it definitely slows things down on hardware. We were losing 1-2 notes of polyphony on RP2040 just by having channels configurable at runtime. But maybe things have changed in compilers/arduino versions and it's worth a re-think.

[jgartrel]

No doubt you all tested this as a runtime arg. I just don't see how the compiler could fully optimize in such a case.

Also I want to include a reference to how to extend a core and build properties to include such defines for future people that go down this path (assuming this pull request is accepted): arduino/arduino-cli#1684 (comment)

[jgartrel]

Here is an example of how one might adjust AMY_SAMPLE_RATE via a global include override from a custom board definition using the code changes suggested by this pull request.

1. Create a global include header file:
   https://github.com/jgartrel/amy_synth_nrf52_example/blob/22e341aff6e8ed8df65f54e289eda773161b0535/global_include/global_macros.h#L1-L8

2. Reference that header file in a custom board definition:
   https://github.com/jgartrel/amy_synth_nrf52_example/blob/22e341aff6e8ed8df65f54e289eda773161b0535/hardware/shorepine/nrf52/boards.txt#L153

No other changes to the original library or core need to be done to make this work. Please note, replacement header files could also be placed in the the global_include directory since that will appear first in the search path. This would allow for a whole header file to be replaced if necessary. Obviously, one could also supply the override defines directly in the board definition and forego step number for the simplest cases.

I think this pull request could provide a good alternative for the AMY project given that the required optimizations for AMY would likely be impossible if these parameters were passed at runtime.