-
-
Notifications
You must be signed in to change notification settings - Fork 98
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
Project: document codebase, outline restructuring and code guidelines #2889
base: community
Are you sure you want to change the base?
Changes from 4 commits
3725553
780e4ab
4023736
a6bd834
bdb39f8
0c0e474
e732bb8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
# Code Documentation | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
# Proposed Code Guidelines | ||
|
||
## Naming Conventions | ||
### Menu Items | ||
- Menu item strings for oled should be descriptive. *F.e.: `delay divs on golden knobs` as opposed to `alternative golden knob delay params`* | ||
- As much as possible, menu item strings for oled should fit on the display without scrolling to keep the menu scrolling experience as fluent as possible. *F.e.: `stutter quantize` versus `stutter rate quantize`. The same information is conveyed while avoiding a scrolling menu string.* |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
# Deluge Community Firmware Github Directory Reference | ||
- [Deluge Community Firmware Github Directory Reference](#deluge-community-firmware-github-directory-reference) | ||
- [Main Folder](#main-folder) | ||
- [Source Folder](#source-folder) | ||
- [Deluge Folder](#deluge-folder) | ||
- [GUI Folder](#gui-folder) | ||
- [UI Folder](#ui-folder) | ||
- [Views Folder](#views-folder) | ||
- [Model Folder](#model-folder) | ||
|
||
# Main Folder | ||
`DelugeFirmware/` | ||
|
||
*Contains the main README.md file and several files and directories related to managing the repository and building the software.* | ||
|
||
Important folders: | ||
| Folder | Contents | ||
|-|- | ||
| docs | Contains documentation for using the Deluge | ||
| pages/public | Contains the public website for the community firmware | ||
| [src](#source-folder) | Contains the source code | ||
|
||
# Source Folder | ||
`DelugeFirmware/src/` | ||
|
||
*Contains all source code. Most functional code lives in subfolder [`deluge/`](#deluge-folder). Everything else is managing low-level systems, global definitions, etc.* | ||
|
||
# Deluge Folder | ||
`DelugeFirmware/src/deluge` | ||
|
||
*Main directory for firmware code. `deluge.cpp` runs the boot sequence and main loop* | ||
|
||
| Folder | Contents | ||
|-|- | ||
| drivers | mostly deals with managing hardware | ||
| dsp | digital signal processing, manages audio effects | ||
| [gui](#gui-folder) | graphical user interface, manages all user-facing behavior of the Deluge | ||
| hid | human interface device, manages basic functionality of knobs, buttons, pads, LEDs, display | ||
| io | input/output, manages debug-logging and incoming midi | ||
| memory | memory management | ||
| [model](#model-folder) | manages model_stack, the model Deluge uses to represent and pass around its internal state | ||
| modulation | manages internal modulating of parameters, including automation and midi | ||
| playback | manages playback and recording modes | ||
| processing | manages various processes: audio, CV and live audio engines, sound instruments, metronome, stem export | ||
| storage | manages using files from the SD card | ||
| testing | automated tests | ||
| util | collection of general utilities | ||
| version | deals with the code version | ||
|
||
## GUI Folder | ||
`DelugeFirmware/src/deluge/gui` | ||
|
||
*Graphical User Interface. This folder manages all user-facing behavior of the Deluge and is therefor the main focus for fixing bugs and developing new features* | ||
|
||
| Folder | Contents | ||
|-|- | ||
| colour | colour definitions | ||
| context_menu | the context menu pops up on top of the regular menu, depending on contextual actions | ||
| fonts | font definitions | ||
| l10n | list of strings used in the ui for both 7SEG and oled, can be used for localization | ||
| menu_item | definitions for the options of all menus | ||
| ui | manages the foundational layer of the ui including menus, browsers, keyboard views and saving/loading. `ui.cpp` serves as a base-class that the different views/modes of the Deluge are built on | ||
| [views](#views-folder) | manages the main views/modes of the Deluge, such as clip view and arranger view | ||
| waveform | manages the waveform view | ||
|
||
### UI Folder | ||
`DelugeFirmware/src/deluge/gui/ui/` | ||
|
||
*User Interface. Many low-level and base methods of the user interface are defined here, and reused in higher level code* | ||
|
||
| Class | Relevance | ||
|-|- | ||
| `AudioRecorder` | is a `UI` | ||
| menus | Handles the main menu system. There's no overarching class here | ||
| `QwertyUI` | is a `UI` that uses the grid pads as a qwerty keyboard | ||
| `RootUI` | is a `UI` | ||
| `SampleMarkerEditor` | | ||
| `Slicer` | | ||
| `SoundEditor` | | ||
| `UI` | Absolute base class of all UIs | ||
|
||
### Views Folder | ||
`DelugeFirmware/src/deluge/gui/views` | ||
|
||
*This folder manages all the main modes of the Deluge ui* | ||
|
||
Each of the `final` views - that is, each view that is not meant to be a base-class for something else - defines a global variable of itself at the bottom of its header file. For example: | ||
```cpp | ||
extern AutomationView automationView; | ||
``` | ||
This global variable is reused anytime that view is activated. | ||
|
||
| View | Info | ||
|-|- | ||
| `ArrangerView final` | is a `TimelineView` representing the arranger view | ||
| `AudioClipView final` | is a `ClipView` for audio clips | ||
| `AutomationView final` | is a `ClipView` that handles the automation view in different contexts | ||
| `ClipNavigationTimelineView` | is a `TimelineView` | ||
| `ClipView` | is a `ClipNavigationTimelineView` set up to handle a single clip | ||
| `InstrumentClipView final` | is a `ClipView` for instrument clips | ||
| `PerformanceSessionView final` | is a `ClipNavigationTimelineView` that implements performance fx view | ||
| `SessionView final` | is a `ClipNavigationTimelineView`. This is *Song View* and contains both *Song Row View* and *Song Grid View* | ||
| `TimelineView` | inherits from `RootUI` and `UI`, and is itself a base class for all other views | ||
| `View` | defines a global variable `view` which represents a layer of the user interface which is always available, regardless of what specific view we're in | ||
|
||
### Model Folder | ||
`DelugeFirmware/src/deluge/model` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
# Community firmware organization | ||
#### To work out | ||
- docs folder, (dev/bug_fixes) cleanup, consistency, how-to-use | ||
- what functionality is in hid vs gui and why? | ||
- why is colour in gui | ||
- why is waveform not in ui or views? | ||
|
||
### Naming conventions | ||
#### General observations | ||
- Naming conventions sometimes don't align between the public UI and the code. Song view being called session_view in code is a good example. These differences are also not documented. This makes it difficult for especially people new to the project to find their way | ||
- Some features could have more descriptive names. For example green/blue mode in Grid View are sometimes called launch and edit mode, but are in code and documentation referenced as green and blue. It would be helpful for both users and developers to refer to features by a descriptive name, both in code and documentation | ||
- Some new menu-items - especially in `community firmware` - don't have descriptive names but are listed as "alternate [x] behavior", which puts the load on the user's memory to remember what they do. It would be helpful to have all menu names be descriptive | ||
- At the same time, many menu items are longer than the display allows. This is probably not possible to avoid altogether, but navigating a menu with items that fit the display is significantly faster. It could pay off to rework menu items' naming to be both descriptive and short. For example: `stutter quantize` conveys as much information as `stutter rate quantize` while still fitting on the display. *(Note: some longer names from the community features menu will get shorter automatically when they are implemented in their dedicated spot in the regular menu, f.e. `grid view loop layer pads` will become `loop layer pads` when placed in the grid view menu)* | ||
#### Modes vs Views vs Layouts | ||
If there is a learning curve to the Deluge, I feel the biggest hurdle is the lack of distinction between the scope and significance of different views. Currently, everything from the arranger, to the grid, to sequencing, to audio recording, to loading files is called a **view**. Having no structure to place all these fundamentally different features into makes it difficult to build up a good understanding what is happening. | ||
|
||
Having a more descriptive, hierarchical naming convention for different kinds of features makes it easier for beginners to feel at ease with the interface, *and* makes it easier for developers to add features in a way that keeps the interface manageable. This could be part of refactoring ui & views, if we decide to do so. A proposal: | ||
|
||
1. **Modes** describe mutually exclusive fundamental ways in which the device behaves. Currenly this would only apply to **Song Mode** and **Arranger Mode**. At any time, either one or the other is active - never both. This allows us to rename the Song Mode layouts to **Rows View** and **Grid View**. *This could be implemented without changing anything else.* | ||
2. **Views** exclusively describe ways of **viewing** and **interacting** with a **mode**. Currently this would only apply to **Rows View**, **Grid View** and **Performance View**. *(One could imagine a grid view for arranger mode, where the arrangement plays top to bottom, which would make all views compatible with both modes.)* | ||
3. **Editors:** clip and automation view operate fundamentally different from the more similar aforementioned views. To distinguish them from those views, they are labeled **Clip Editor**, **Clip Automation Editor** and **Arranger Automation Editor**. | ||
4. **Tools:** There is a selection of mostly autonomous tools/features that can be called in different places in the UI, but operate largely the same regardless of where they are called. Think of the browser, waveform editor and audio recorder. These currently aren't called views, but they are documented in the manual under the views they appear in. Instead they should be documented as what they are: a clearly delineated, autonomous tool. *There may be a better name for this.* | ||
|
||
The documentation structure would look something like this: | ||
- General UI | ||
- Browser | ||
- ... | ||
- Modes | ||
- Song Mode | ||
- Arranger Mode | ||
- Views | ||
- Rows View | ||
- Grid View | ||
- Performance View | ||
- Clip Editor *(gets its own main header because of how much happens here, and has a dedicated ui button)* | ||
- Sequencing | ||
- Modulation | ||
- ... | ||
- Tools | ||
- Automation Editor | ||
- Waveform Editor | ||
- Audio Recorder | ||
- ... | ||
|
||
|
||
### Code Cleanup | ||
#### Views | ||
- The current general strategy is to create global variables for all the main views and reuse them when activated. Is it safer/easier to refactor them to static properties? | ||
- None of the views inherit from view.h, which could be assumed would be the case from how the files are named. Instead all views inherit from `TimelineView` | ||
- `ClipNavigationTimelineView` is a `TimelineView` and only implements a few methods. `TimelineView` itself is only used as a base class for `ArrangerView`. This could probably be simplified into one class | ||
RJ-Eckie marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- **Proposal:** | ||
- Combine `TimelineView` and `ClipNavigationTimelineView` to a new `View` class. This is a `RootUI` and will remain the base class of all other views. Shared behavior for views lives here | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In principal I agree with this refactoring approach of having a base view class from which all other view classes inherit. To improve understanding of the implications of all this it would probably be worth searching for instances of getRootUI(), getCurrentUI(), changeRootUI(), &instrumentClipView, &automationView, etc. as it will give a better appreciation of how embedded those concepts are throughout the codebase. In terms of refactoring view classes, it's definitely something we've talked about a lot. InstrumentClipView is the original big "catch all" class for interacting with melodic instrument and kit instrument clips. A lot of code could definitely be factored out to make the view code easier to comprehend. If I give an example of automation view, which I initially built by taking a copy of instrument clip view and building the automation view functionality on top of, it's now a very huge beast of a class that needs to be refactored as it's just too big to manage now. I want to refactor its current state so that automationView is a base class that other classes that take care of automationView specific functionality can inherit from / override. e.g. Note Velocity Editor and Parameter Automation Editor are both part of the automationView class but they should be pulled out into their separate classes. When I started looking at it I started looking at splitting things out like this: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I definitely intend to spend some more time getting an understanding of the UI classes and how everything is implemented, before starting any refactoring work. Documenting the UI folder is next on the list. I feel we have a lot to gain here in both accessibility and possibly stability/speed. |
||
- The current `View` holds three types of behavior, which should all be relocated to their appropriate locations: | ||
- View-specific behavior moves to their respective views, where possible as overrides from the new `View` | ||
RJ-Eckie marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- Shared behavior between all views moves to the new `View` class | ||
- Global UI behavior really doesn't belong in the views folder at all, should probably move to `src/ui/ui.cpp` | ||
- *I have yet to dive into the code rigorously enough to realise whether or not there are any code-breaking issues that prevent this solution* | ||
- *Song View* is called `SessionView` in the repo | ||
- *Song Row View* and *Song Grid View* both live in `SessionView` and are probably different enough to live in their own separate files. | ||
- **Proposal:** | ||
- Rename `SessionView` to `SongView` | ||
RJ-Eckie marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- Separate unique behavior for *Song View, Row Layout* and *Song View, Grid Layout* into `song_view_grid_layout.h` and `song_view_row_layout.h` | ||
- Rename `PerformanceSessionView` to `PerformanceView` | ||
RJ-Eckie marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- Refactor methods and properties accordingly | ||
- `PerformanceSessionView` is not a `SessionView` | ||
- **Proposal:** | ||
- Rename `PerformanceSessionView` to `PerformanceView` | ||
- `AutomationView` is too large to manage | ||
- **Proposal:** | ||
- Refactor `AutomationView` to a base-class which can be inherited from in different contexts, behavior can be organized in child-classes | ||
|
||
#### UI | ||
- There's a whole bunch of global stuff here. It would probably help legibility in other parts of the code if these were captured in a (static) class. UI mode defines could be in an enum : uint32_t for the same reason. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
# UI | ||
Absolute base class for all UI. Implements base virtual methods for all user actions, which get overridden in sub-classes. | ||
```cpp | ||
virtual ActionResult padAction(int32_t x, int32_t y, int32_t velocity); | ||
virtual ActionResult buttonAction(deluge::hid::Button b, bool on, bool inCardRoutine); | ||
virtual ActionResult horizontalEncoderAction(int32_t offset); | ||
virtual ActionResult verticalEncoderAction(int32_t offset, bool inCardRoutine); | ||
virtual void selectEncoderAction(int8_t offset); | ||
virtual void modEncoderAction(int32_t whichModEncoder, int32_t offset); | ||
virtual void modButtonAction(uint8_t whichButton, bool on); | ||
virtual void modEncoderButtonAction(uint8_t whichModEncoder, bool on); | ||
``` | ||
Plus a whole bunch of general, rendering, conversion (etc.) virtual methods. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like where this is going. We're definitely on the same page regarding Modes (Song vs Arranger) and the Views to view and interact with the modes.
Regarding the editor concept. I think this term makes sense to me. You are basically editing a component of the project.
Regarding Automation View tho, it's a bit complex of a beast in the sense, it has an "Overview" component which basically acts as a "Report" / "Launchpad" so to speak which can bring you into the Editor.
Also the automation view currently has two UI's for editing parameter automation as well as note velocity. Those are definitely editor's and I agree with the term editor there:
We'll also eventually have other editors too:
So I'm not entirely sure how to present that to a user.
The way I see it, it's a UI for interacting with and editing various parameters, whether they be modulation or note parameters.
I think the term Automation is familiar to users when it comes to editing parameters like this.
When it comes to editing clips, we basically have 3 editing layouts:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re: automation view - see following commit
Re: editing clips - from a programmer's perspective, layouts would be a good label here. From the user's perspective, I would just call it "Editing audio clips in the clip editor." As they don't have any say over what layout they get, we might as well not label it