edgetx
edgetx copied to clipboard
EdgeTX
Option to define model notes file name from model setup (#1396)
Implements #1396
Summary of changes: Added option to define model notes file from the model setup screen. If the file is defined it is used. If it is not defined - modelName.txt is used. If this is not present - modelXX.txt is used.
Changes for YAML storage and translations are done. Tested on TX16S and NV14 for color LCD and on simulator for X9d+ and X7.
Changes to Companion required otherwise the setting will be lost and also need to edit
@elecpower Can you be more specific, please? I tested it on simulator and the file name was saved in modelXX.yml. I did not try copy from Companion to TX though.
@eshifri - if the setting is not present in the companion - it will not be displayed and probably would be lost when transferring settings from companion to radio.
@elecpower Can you be more specific, please? I tested it on simulator and the file name was saved in modelXX.yml. I did not try copy from Companion to TX though.
@eshifri, @piotrva is correct. Companion needs to be made aware of every field in the yml files otherwise it:
- will not be able to display and/or edit; and
- as the yml files are written in full, will not write the unknown field to the yml files and then we get issues raised of Companion corrupting yml files
@elecpower Can you please take a look at the last commit? I'm definitely missing something since the new function on_notesName_editingFinished() is never called. Thank you.
Reason: as you are using Qt default signals and slots, the field name part of the slot name must match exactly the field name in the ui definition along with the event signal.
For code consistency can you please use camel case and no underscores exception when using Qt's default slot names. Also ui field label names are okay to default to Qt Designer auto generated if not referenced in code otherwise preferable to use ui edit field name and append Lbl. This makes it easier for future maintenance to identify the pair in the code and ui definition. There are numerous exceptions that have been created over time and it makes my life challenging.
Get back to me if this doesn't make sense or you need some more help.
Thank you @elecpower , but this is a little bit above my knowledge level. I understand that you prefer to have modelNotes and notesFile (my preference as well; I think I have just saw the underscore names in the code).
The part I do not understand is "field name part of the slot name must match exactly the field name in the ui definition along with the event signal". Do you mean the function name is supposed to be void SetupPanel::on_notes_file_editingFinished() ?
Thank you for the help; much appreciated.
I've finally caught up with rebasing this PR... (hopefully didn't screw something up there... it was a fun one) as it had several conflicts due to changes to companion UI code and yaml data structs, etc. From a quick test on the TX16S, it seems fine... I'll play a little more with that but don't expect to find any breakage. Then onto B&W and syncing with companion.
@elecpower I made a change (last commit) around populating the filename line-edit field with either configured value or if that is not present, the model name, but I couldn't figure out how to connect the model name field to the notesFile field so that a change of the model name would update the field? It is handled correctly behind the scenes as the edit button reflects the new name, but I was trying to make it so it tells the user what the generated filename would be... you know, be helpful like :grin:
@pfeerick The original implementation fro Companion assumed that the file exists and can be just selected. If you allow to enter a non-existing name, some new problems may suffice.
@pfeerick need to trigger update of notesFile from on_name_editingFinished. However I assume we would not want to update the notesFile if it wasn't blank and not the same as the name field before the name field was updated.
An additional enhancement to this PR could be to have the ability to browse the profile sd card for checklist/notes files and handle the configured one not existing such as when the sd card hasn't been synced with the radio like is done for model image ;-)
@pfeerick The original implementation fro Companion assumed that the file exists and can be just selected. If you allow to enter a non-existing name, some new problems may suffice.
Checked and if it doesn't exist Companion will create then it comes down to syncing profile sd card with radio.
need to trigger update of notesFile from on_name_editingFinished. However I assume we would not want to update the notesFile if it wasn't blank and not the same as the name field before the name field was updated.
An additional enhancement to this PR could be to have the ability to browse the profile sd card for checklist/notes files and handle the configured one not existing such as when the sd card hasn't been synced with the radio like is done for model image ;-)
@elecpower Coming back to this PR now... I see what you mean with the on_name_editingFinished trigger. And since it becomes a bit circular (since you need to keep track of the state of both on entry, and then make the decision on finish)... I think what you were getting at that perhaps not a good idea?
If the QInput field was changed to a combo box similar to the model image browser, and the first entry was ... 'Default' (and used the model name) ... would that be better? After all, on the radio, you can only pick from the list on it also, so it's not like you can enter a name there.
Ok, thanks. I'll revisit this on the weekend. My thoughts on this after mulling it over is that the combobox default (index 0) would reflect the name of the model name as it is entered, and would dynamically update if the name is change, thus reflecting the filename the radio will be looking for. If the user chooses any other entry, well, naturally the file choosen via the file browser capability is used.
We can also add an option to assign the notes file from the Radio Setup->SDcard->file menu simo;ar to the image file. So the user can look into the content first. Of course this is not relevant for the companion.
Good point, that would be nice to have also... :) Also some people may want to use the SD browser instead of via the model settings...
Don't worry about the rebase, that will be my problem once 2.8.0 is safely on it's way ;) This will happen, was just pushed to 2.9 because there are never enough hours in the day to play with the stuff you want to. 😢
This one and custom warning are most requested from my side, so I am looking forward to test them once they will be in 2.9 nightly :)