Skip to content

Conversation

@krskajo1
Copy link

@krskajo1 krskajo1 commented Jan 18, 2026

Hello,
this PR adds support for the missing Mustang I V2 effects (Ranger Boost, Greenbox, Orangebox, Blackbox, Big fuzz) and modulations (Wah, Touch Wah, Diatonic Pitch Shifter). The support includes default effect and effect windows.
Main change of this commit is in the ID of individual effects. It seems that they are in fact 16-bit wide, instead of current 8-bit width. This mainly influenced the function lookupEffectById, where I added second parameter uint8_t idmsb with implicit value of 0.

The newly added effects are currently appended to the bottom of the combobox list in the effect window, as their position must correspond with their value in the effects enum.

Additionally I've noticed there is a lot of duplicate code, especially in default effects and effects windows regarding knob labels. I know there might be a good reason for it, such as proper shortcut function, however, I think that by doing this line-efficiently would benefit legibility of the code and make editing it easier.
One solution I come up with and implemented for the new effects utilizes maps to store knob description for given knob like so

mapLbls["Brightness"] = UIText{
  tr("&Brightness"),
  tr("Effect's %1 \"Brightness\" dial").arg(slot.id() + 1),
  tr("Allows you to set \"Brightness\" parameter of this effect"),
  tr("Effect's %1 \"Brightness\" box").arg(slot.id() + 1),
  tr("Allows you to precisely set \"Brightness\" parameter of this effect")};

and sets for an effect

case effects::RANGER_BOOST:
  setTexts(ui.get(),
  mapLbls["Level"],
  mapLbls["Gain"],
  mapLbls["Low"],
  mapLbls["Brightness"],
  mapLbls["Disabled"],
  mapLbls["Disabled"]);
break;

Alternatively, creating a class for each effect (deriving from effect base class), which would define its labels, is also an option.

Please, let me know if you do or don't agree with the map approach and or anything in this PR and what should be changed. I will gladly make the necessary edits.
Best regards
Joe

@krskajo1
Copy link
Author

I realized I forgot to implement and run tests. I will implement them and update the PR.

@krskajo1 krskajo1 marked this pull request as draft January 18, 2026 14:30
@krskajo1 krskajo1 marked this pull request as ready for review January 18, 2026 14:53
@krskajo1
Copy link
Author

krskajo1 commented Jan 18, 2026

Tests and formatting corrected. This PR is now ready for review.



constexpr effects lookupEffectById(std::uint8_t id)
constexpr effects lookupEffectById(std::uint8_t id, std::uint8_t idmsb = 0)
Copy link
Owner

@offa offa Jan 22, 2026

Choose a reason for hiding this comment

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

We should consider expanding the id type to uint16_t instead. Since it's 0 for other devices, this should be safe.

Copy link
Author

Choose a reason for hiding this comment

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

I am totally for changing the type to uint16_t, adding a second parameter was just a less intrusive change.
Should I change it in this PR or create another one?

Copy link
Owner

Choose a reason for hiding this comment

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

Let's keep working on this PR. No need to create an additional one.

Copy link
Author

Choose a reason for hiding this comment

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

Ok. The change is now implemented.

@offa
Copy link
Owner

offa commented Jan 22, 2026

Nice, thank you! 👍

…ded decodeEffectsFromDataEffectsValues test with Mustang I V2 effects.


constexpr effects lookupEffectById(std::uint8_t id)
constexpr effects lookupEffectById(std::uint16_t id)

Check warning

Code scanning / CodeQL

Poorly documented large function Warning

Poorly documented function: fewer than 2% comments for a function of 101 lines.
Copy link
Owner

@offa offa left a comment

Choose a reason for hiding this comment

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

I gave it a quick test on a v1 device and the v2 effects are listed there too. They should be limited to Mustang I v2 only though.

Also I'd suggest to keep the UI entries sorted and grouped by family. At the moment it's all v1 models, and then the v2.

connect(ui->pushButton, SIGNAL(clicked()), this, SLOT(get_settings()));
connect(ui->pushButton_2, SIGNAL(clicked()), this, SLOT(save_default_effects()));

// Up to discussion, prepare map with default effects description to greatly
Copy link
Owner

Choose a reason for hiding this comment

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

Eventually we should improve the complete text handling here. Ok for now though.

@krskajo1
Copy link
Author

I gave it a quick test on a v1 device and the v2 effects are listed there too. They should be limited to Mustang I v2 only though.

Also I'd suggest to keep the UI entries sorted and grouped by family. At the moment it's all v1 models, and then the v2.

Ok, I'll have a chance to look into the displaying the valid effects in a week or two.

By grouping UI elements do you mean e.g., the combo boxes? That the entries should be grouped by their type (stomp box, modulation, delay, reverb)?

@offa
Copy link
Owner

offa commented Jan 28, 2026

By grouping UI elements do you mean e.g., the combo boxes? That the entries should be grouped by their type (stomp box, modulation, delay, reverb)?

Exact! The combox list entries grouped by [1], [2] etc. in the UI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants