Skip to content
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

fix(cpn): inputs and mixes must have source #5798

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

elecpower
Copy link
Collaborator

@elecpower elecpower commented Jan 17, 2025

Fixes #5793

Summary of changes:

  • check every model's inputs and mixes for source
  • new traffic light status for each models and settings file, green if no errors
  • if no source(s):
    • only allow saving to etx format
    • disable writing to radio or sd path as no source causes unexpected behaviours
    • disable simulate model and radio for the same reasons
    • new dialog to display errors
    • change status to red and display the number of models with errors

@elecpower elecpower added bug 🪲 Something isn't working companion Related to the companion software labels Jan 17, 2025
@elecpower elecpower added this to the 2.11 milestone Jan 17, 2025
@elecpower elecpower marked this pull request as draft January 17, 2025 18:56
@elecpower
Copy link
Collaborator Author

The solution is not as simple as I first thought.

Just as in the firmware when an input or mix is added or edited, via the ui, the source cannot be none. So no issues should arise here.

The issue mainly occurs when the Models and Settings are read that are not compatible with the current radio profile. Companion preserves the invalid lines to enable the user to manually fix them rather than have to create whole lines from scratch. This introduces the possibility for errors and requires extra effort.

Checking and prompting when models are written would apply to Write Models and Settings to Radio, Simulate Radio, Simulate Model, Export Model, Save Models and Settings and Save As Models and Settings.

Also we cannot just drop inputs and mixes with no source as any references to those dropped lines would also break. Therefore the references would need to be fixed creating more effort.

Prompting when invalid lines are found and then if proceeding, dropping invalid lines and fixing references would be very annoying in situations say where the user is progressively converting models from one radio to another. Requiring the user to fix every model before activating any of the above features would not be acceptable.

Alternatively users could open the incompatible Models and Settings, create a new Models and Settings file, then copy models one by one to the new file fixing as they go to avoid the prompts and dropped lines issues. A backward step from what they have now.

Writing to etx files could remain unchanged, Write to Models and Settings to Radio could enforce fixing, however the biggest hurdle is Simulations.

I'm unsure of the effort or complexity but only valid models can be simulated ie invalid ones would not show in radio simulator. If practical could also apply to Write Models and Settings to Radio.

Thoughts?

@philmoz
Copy link
Collaborator

philmoz commented Jan 17, 2025

How about if any model in the list has invalid entries:

  • Show the model info in the list in red to indicate it has a problem (like we show in italic for modified entries).
  • Clicking 'Simulate Radio' or 'Write to Radio' buttons pops ups warning that the operation can't be done because there are invalid models.

Basically disallow operations that would result in errors in either radio or simulator.

Might also help if the invalid lines could be highlighted when editing the model.

@elecpower
Copy link
Collaborator Author

elecpower commented Jan 18, 2025

Might also help if the invalid lines could be highlighted when editing the model.

That might be okay for inputs and mixes due to the widget being used but not so easy for errors in other settings. I would favour a user instigated pop up dialog which lists all the errors and warnings. Add to context and model menus.

@pfeerick
Copy link
Member

I would favour a user instigated pop up dialog which lists all the errors and warnings.

This sort of workflow?

  1. Take a model/radio settings configuration that is invalid with current profile.
  2. Get the initial conversion issues warning. Maybe have the option to skip / not apply corrections?
  3. Carry on your merry way... perhaps coincidentally fixing some of those invalid settings or not.
  4. Clicking 'Simulate Radio' or 'Write to Radio' buttons pops ups warning that the operation can't be done because there are invalid models/settings. (Phil's point above). Maybe a "Show Invalid" button that retriggers the validation/conversion checks.
  5. You can keep retriggering the conversion checks / "Validate settings?" manually to keep bringing up the list of things that still need correcting.

@elecpower elecpower removed this from the 2.11 milestone Jan 22, 2025
@elecpower elecpower force-pushed the elecpower/cpn-fix-no-input-no-src branch from 9f78e69 to e637725 Compare January 24, 2025 05:33
@elecpower
Copy link
Collaborator Author

I have two ways of preventing invalid models being written or simulated.

  1. as suggested ie message dialog; or
  2. disable the menu and toolbar options though downside is no pop up message to explain why actions not available albeit the model name would be coloured red.

Should the invalid/error colour be user selectable to cater for colour blind users?

Feedback

@elecpower
Copy link
Collaborator Author

The latest commit takes the second option

@elecpower
Copy link
Collaborator Author

elecpower commented Jan 24, 2025

I've included 'Error' prefix as the information dialog as it could also be used to display warnings eg Flight mode has no switch assigned in future enhancement.

Anyway presented for your feedback.

annoted

@elecpower
Copy link
Collaborator Author

elecpower commented Jan 25, 2025

There is an issue with unsetting the model's errors flag after the errors are fixed and the edit dialog is closed.
Models flagged correctly on save and reload.

Update: Fixed

@pfeerick
Copy link
Member

Maybe a (permanent?) message/zone could be added to the status bar regarding validity of the model config? And you had to ask about configurable colour, didn't you... sounds like you want more work!🤣

And playing devils advocate, how well will that dialog scale if say there are a dozen models, and 2-3 issues with each?

@elecpower
Copy link
Collaborator Author

elecpower commented Jan 25, 2025

Maybe a (permanent?) message/zone could be added to the status bar regarding validity of the model config? And you had to ask about configurable colour, didn't you... sounds like you want more work!🤣

Would need to think about message/zone as maybe too small as the window is resizeable. Also the invalid models are coloured.
The colour was around have we been asked to cater for the impairment otherwise wait until feature requested. I do not need anymore work but when you are in the area then sometimes tack on a little extra.

And playing devils advocate, how well will that dialog scale if say there are a dozen models, and 2-3 issues with each?

The dialog is for the currently selected model

@elecpower
Copy link
Collaborator Author

As it is possible to have multiple files open then the status would need to be displayed in each file window. I can see a situation where one or more errant models are not visible when there are many so it would not be obvious why functions are disabled. I'll see what I can do.

@elecpower
Copy link
Collaborator Author

elecpower commented Jan 26, 2025

Status bar added to each models and settings file dialog. Could be extended to highlight warnings with orange light.

error

ok

Updated information dialog

Screenshot from 2025-01-27 09-14-15

@elecpower elecpower marked this pull request as ready for review January 27, 2025 10:02
@elecpower elecpower changed the title fix(cpn): inputs and mixes must have source when saving fix(cpn): inputs and mixes must have source Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working companion Related to the companion software
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mixer without source, kills following mixes (no more output)
3 participants