Client: Pass through flag for ctrlmidich used

[pljones]

Short description of changes

Rather than changing the name of the connecting client (and losing the actual name), based on whether --ctrlmidich is used, pass that information to the fader itself.

Also, to partly undo the damage, attempt to recover fader details previously saved with the channel number prefix.

CHANGELOG: Client: Bugfix: Remove channel number from saved fader tag when --ctrlmidich is in use. In this release --cleanuplegacyfadersettings can be used on any saved fader settings that have been corrupted to attempt recovery.

Context: Fixes an issue?

Fixes: #2680

Does this change need documentation? What needs to be documented and how?

No.

Status of this Pull Request

I've tested it local. The following should be borne in mind:

• This does not remove entries from Jamulus.ini (it should match the old ones, but they'll still be there, cleaned values will get written).
• I've not tested fader sort options.

What is missing until this pull request can be merged?

User test: @pcar75 and @rdica - can you take a look, please?

Checklist

• I've verified that this Pull Request follows the general code principles
• I tested my code and it does what I want
• My code follows the style guide
• I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
• I've filled all the content above

[ignotus666]

Won't be able to test until tomorrow but will do so.

[pcar75]

Bonjour,

What is missing until this pull request can be merged?
User test: @pcar75 and @rdica - can you take a look, please?

Sorry I missed @pljones message above ... first time I participate in a project (since retirement he he :-).

Before any tests , I need to know that I could - and how I would - revert to my current Jamulus version ?
Given that , I can test in my Windows 10 env ... not my new Ubuntu Studio env as it is still somewhat unstable.

• Where do I get the modified version and how to install it ?
• Any specifics tests/procedures you would like me to run ?
• Where and how do I report results ?
• By what date & time are results expected (latest) ?

Thanks

Tests scenarios considered :

-Not broken :
01- Make a backup of original jamulus.ini (populated with fader info)
02- Execute jamulus (no ctrlmidich) and connect to a session with some known fader tag (user)
03- Modify 1 info (level) about it's fader
04- Use Save Mixer Channels Setup (to IETest01.jch )
05- End session and exit jamulus
06- Compare new and old jamulus.ini; verify that only 1 fader info was changed and that fadertags do not contain channel number.
07- Compare IETest01.jch and old jamulus.ini; verify that only 1 fader info was changed and that fadertags do not contain channel number.

-Currated1
01- Restore backup of original jamulus.ini
02- Modify jamulus shortcut to add -ctrlmidich "1:o2" argument
03- Execute jamulus on a session with some known fader tag
04- Modify 1 info (level) about it's fader
05- Use Save Mixer Channels Setup (to IETest02.jch )
06- End session and exit jamulus
07- Compare new and old jamulus.ini; verify that only 1 fader info was changed and that fadertags do not contain channel number.
08- Compare IETest02.jch and old jamulus.ini; verify that only 1 fader info was changed and that fadertags do not contain channel number.

[pljones]

Hi @pcar75

Where do I get the modified version and how to install it ?

Open the "Checks" tab on this pull request in a new browser tab then select the "Auto-Build" group. You'll see a number of "Artifacts". Pick whichever you need -- it will be a ZIP containing the normal install file for that platform.

When installing, I'd recommend using a separate folder, then "reverting" isn't a problem (the windows installer will remember this new location as the default for your next install but nothing else will care).

Never use one of your "real" inifiles for test purposes - back them up (just in case) and use copies (not the backups, fresh copies...).

For test purposes:

• run 3.9.0 with a clean --inifile (either take a copy and manually just delete all the fader details or start afresh)
  • save to a new save file
  • check the bug is there
  • check how fader sorting works
  • exit the app and see the inifile has the bug, too
• run the build from this pull request (again, either a copy of your 3.9.0 inifile with the fader details removed or a fresh one)
  • save to a new save file
  • check the bug fixed
  • check how fader sorting works - it should be the same... hopefully...
  • exit the app and see the inifile is also good
• run the build from this pull request (ideally a copy of your 3.9.0 inifile with the buggy fader details)
  • take a copy of a buggy save file
  • load the settings from the copy
  • save to a new save file
  • check the bug is fixed
  • exit the app and check the ini file is now sane

[pcar75]

@pljones
Upgraded to 3.9.0. Tested that incorrect behavior is still there with jamulus.ini , fader sorting (by name is by "ChannelNumber:Name") and Export (same as .ini file).

But, per your instruction, I cannot find a downloadable file of your fix https://github.com/jamulussoftware/jamulus/runs/8226558392?check_suite_focus=true

[pljones]

You haven't followed the instructions.
Click: https://github.com/jamulussoftware/jamulus/pull/2839/checks
Click on ("Auto-Build", not the ">")
Scroll down and find the build you want:

The download will be a ZIP.

[ignotus666]

Just did some testing and it seems to work as advertised. An old .ini file with the bug gets written over with correct fader tags (no prepending with a number) and sorting seems to be correct too.

[pcar75]

run the build from this pull request (again, either a copy of your 3.9.0 inifile with the fader details removed or a fresh one)

Test Ok : --ctrlmidich does not make Jamulus write ini file with ChannelNumber,
ImportExport file also OK.
Sort by Name does not sort by ChannelNumber.

run the build from this pull request (ideally a copy of your 3.9.0 inifile with the buggy fader details)

Test Ok : buggy ini with Jamulus no --ctrlmidich is rewritten without ChannelNumber.
ImportExport file also OK
Sort by Name Ok

[pljones]

Thanks both!

[pljones]

Move into the correct scope - inside the if.

[pcar75]

Bonjour,
Shouldn't there be a test with a real MIDI device ? (I don't have one.)

[ignotus666]

I tested it with a MIDI device - works fine.

[pljones]

• Maybe also document the special flag as part of the Changelog with a note saying that this flag is only required once and is about to be removed in an upcoming version again.

I've added it to the "changelog" tag on the PR - I've tried to keep it short but it can be edited/formatted better when we get there.

[ann0see]

Since we agreed to remove this soon, I'd prefer to document it in the help text, and remove it later on.

[pljones]

It's going to be totally meaningless to most users. To those for whom it's applicable, they'll use it once and never again. I can't see the point of making the help text get even larger than it is now for this.

[ann0see]

As it's a short term feature, and if I wanted to use it, I'd like to know which version has it. If it shows up under --help, the version supports it, else not. It's much easier than going through the changelog on GH for end users.

[pljones]

If it shows up under --help, the version supports it,

If running the app with the option doesn't complain it's not recognised, that means it's supported, too.

Also, if it goes into help, that means we should update the website User Guide, too.

[ann0see]

If running the app with the option doesn't complain it's not recognised, that means it's supported, too.

Fair point.

I'd still like to have it prominently documented somewhere besides the release announcement. I assume most users don't read the announcement at all.

[ann0see]

Probably?
What about printing it to stdout if the --ctrlmidich option is set and to the website as long as it's in?

[pljones]

You mean in main.cpp when it parses --ctrlmidich, output a hint? Desktop client users generally won't see that (on Windows we launch without a terminal attached so output goes "nowhere") and most who use it will have edited their shortcuts, I'd expect.

The people it's likely relevant for are those who'll be searching the internet for a fix for the problem. I'm not sure how well our documentation gets picked up by search engines -- or how best to get new documentation into it so it's found in a timely manner.

[ann0see]

We could write a forum post with a how to?

[pljones]

This all arose in https://github.com/orgs/jamulussoftware/discussions/2678 -- that could have a comment added with how to fix it.

[pljones]

I've noted on the Reminders for the release announcement to comment on the original discussion, so going ahead with the merge here.

[ann0see]

Other than my comment, I'm happy to approve it.