Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new “Level” stage to both the simulation backend and the GUI, allowing users to adjust a gain parameter.
- Backend: Introduces
LevelStageimplementingStagewith a configurable gain. - GUI: Adds
level_widgetfor the Level stage slider, integrates messaging, default config, and chain construction. - Module imports and enums updated to include the new
Levelstage.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/sim/stages/mod.rs | Registered the new level module under stages |
| src/sim/stages/level.rs | Added LevelStage struct and Stage trait impl |
| src/gui/widgets/mod.rs | Included level in GUI widgets registry |
| src/gui/widgets/level.rs | Created UI widget for adjusting Level gain |
| src/gui/amp.rs | Integrated Level in StageType, StageConfig, Message, GUI view/update, and audio chain |
Comments suppressed due to low confidence (4)
src/sim/stages/level.rs:3
- There are no unit tests for
LevelStage. Consider adding tests that coverprocess,set_parameter(including boundary cases), andget_parameterto ensure correct behavior and prevent regressions.
pub struct LevelStage {
src/sim/stages/level.rs:3
- Public API types like
LevelStagewould benefit from doc comments explaining its purpose and the valid parameter range forgain.
pub struct LevelStage {
src/sim/stages/level.rs:36
- [nitpick] The error messages are inconsistent (
"Unknown parameter"vs"Unknown parameter name"). Consider using a single format, e.g.,Err("Unknown parameter: {name}"), to aid debugging.
_ => Err("Unknown parameter"),
src/gui/amp.rs:392
- The pattern here may copy the
LevelConfiginstead of mutating the original. You should match the mutable reference, for example:
if let Some(StageConfig::Level(cfg)) = self.stages.get_mut(idx) {
cfg.gain = gain;
}using StageConfig::Level(cfg) where cfg is a &mut LevelConfig (e.g., ref mut cfg) to ensure the field is actually updated.
if let Some(StageConfig::Level(cfg)) = self.stages.get_mut(idx) {
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new "Level" stage to the amplifier simulation and GUI. The changes include adding support for the Level stage in both the simulation backend and the user interface, enabling users to adjust the gain parameter for this stage. Below is a summary of the most important changes grouped by theme:
Backend Simulation Enhancements:
LevelStagestruct insrc/sim/stages/level.rs, implementing theStagetrait to process audio samples with a configurable gain parameter.src/sim/stages/mod.rsto include thelevelmodule in the simulation stages.GUI Integration:
level_widgetfunction insrc/gui/widgets/level.rsto create the user interface for configuring the Level stage. This includes a slider for adjusting the gain parameter.src/gui/widgets/mod.rsto include thelevelmodule in the GUI widgets.src/gui/amp.rsto integrate the Level stage into the GUI, including updates toStageType,StageConfig,Message, and theAmplifierGuiimplementation to handle Level stage configuration and updates. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]Closes #17