Allen-Synthesis / EuroPi

EuroPi: A reprogrammable Eurorack module based on the Raspberry Pi Pico
Creative Commons Zero v1.0 Universal
420 stars 79 forks source link

Create a firmware library of reusable common functions #275

Open awonak opened 1 year ago

awonak commented 1 year ago

With the growing number of scripts, we are starting to see common functionality that might be beneficial to other scripts. Instead of copying & pasting or importing directly from other scripts, it would be better to have a library of reusable classes and functions.

Since this library is intended to be used by more than just one script, it's important that it has the following features:

1) Included with the firmware package - since any script could potentially use this library code, it should always be present alongside the firmware, not just contrib.

1) Documentation - this will help with discoverability and make it easy to understand what to expect when using the class or function.

1) Tests - tests are important because we need to ensure that we know exactly how the library code behaves and if it ever needs to change, we understand the impact of those changes and how it could affect users of that code.

awonak commented 1 year ago

This issue was inspired by the work in #248 which builds off of the functionality of several excellent preceding scripts. Other script authors have also expressed interest in reusing some of that shared behavior such as the Euclidean Rhythm pattern generator.

We have some precedence in shared code with the Knobs functionality in: https://github.com/Allen-Synthesis/EuroPi/tree/main/software/firmware/experimental

awonak commented 1 year ago

Proposed workflow for script authors from @mjaskula

A potential approach:

mjaskula commented 1 year ago

The firmware package is already a 'library of reusable common functions'. I'd rather that we not propose a place within the firmware package for shared, user contributed code. Instead, I think that we should provide a process by which code can be 'elevated' into the firmware package.

The firmware package should be organized by functionality, not by the source of the code. The features that distinguish code that resides within the firmware package from code without should be things like: stability, usefulness, documentation, tests and our ability to maintain it.

So I would propose that the following is needed to resolve this issue:

roryjamesallen commented 1 year ago

I think that the firmware i.e. europi.py only, should be only for interacting with the hardware itself. Anything else, even if it's very relevant to the fact that it is a module, should be separate, but I agree, probably all in one place, and probably without the name 'experimental'.

I'm not sure what a great name would be I'll have to think, but something along the lines of 'contrib library' - a library where users can submit PRs to add classes and functions which are likely to be useful to other programmers, and which can be bundled with europi.py in every implementation other than a user writing their own script from scratch without using anyone else's code beyond europi.py

I definitely think that people shouldn't be importing things from other contrib scripts, that seems like a recipe for disaster given that scripts do actually get updated (for example the consequencer gets updated pretty regularly) and I would like to keep away from a situation where a small change to one script that might seem innocuous has the potential ability to 'break' other scripts in the contrib folder. It works for now, but I'd like to get to a point where anything people are choosing to import from other scripts now can be condensed down to the most non-specific implementation of whatever the class/function is doing, and then put that in a single big contrib library

roryjamesallen commented 1 year ago

I just started writing some oscillator stuff using @t-schreibs PIOClock library, and then noticed that the class is named differently in the polysquare code, so I think that's a good example of how some things will need to be moved around (with the consent of the original script creators), for example PIOClock would be a class in the new contrib library, and then polysquare.py can import it as SquareOscillator if it wants to keep the name. That way users who aren't using it as a square oscillator can still use the great code and no contrib script has to rely on another

chrisib commented 1 month ago

Given the number of recent additions to the experimental namespace, does this issue need to remain open? Or can we close this and handle any migrations of shared functionality out of contrib scripts into experimental on a case-by-case basis with MRs?

roryjamesallen commented 1 month ago

Given the number of recent additions to the experimental namespace, does this issue need to remain open? Or can we close this and handle any migrations of shared functionality out of contrib scripts into experimental on a case-by-case basis with MRs?

I agree that experimental now performs all the functions we hoped a specific user contributed library would, although I'd be tempted to keep this open until we write up more in-depth documentation and examples of how generic code can be pushed to experimental and imported by individual scripts. Instinctively I want to work out a more objective way of categorising what code belongs in experimental rather than just within a script, but given the creative and sometimes unexpected nature of contributions, it might make more sense to keep the decision making case-by-case