EdgeTX / edgetx

EdgeTX is the cutting edge open source firmware for your R/C radio
https://edgetx.org
GNU General Public License v2.0
1.61k stars 340 forks source link

Radio/Model settings cannot be backed up by Companion in 2.10 firmware version #3758

Closed philmoz closed 1 year ago

philmoz commented 1 year ago

Is there an existing issue for this problem?

What part of EdgeTX is the focus of this bug?

Companion

Current Behavior

Because of the changes to the RADIO.YML file format any attempt to backup/restore settings from a radio using companion will result in a corrupted RADIO.YML file. Companion has not yet been updated so it is now impossible to backup/restore a radio using any version of companion.

More concerning is, in future, if someone upgrades their radio; but does not upgrade Companion they risk corrupting their radio.yml file when attempting to do a backup/restore.

Expected Behavior

Radio can be backed up and and restored using Companion.

Steps To Reproduce

Version

Nightly (Please give date/commit below)

Transmitter

FrSky X10 Express / X10S Express (ACCESS), FrSky X12, FrSky X-Lite / S / Pro, Jumper T12, Jumper T16, Jumper T18, Jumper T-Lite, Jumper T-Pro, Radiomaster Boxer, Radiomaster T8, RadioMaster TX12 / TX12MK2, Radiomaster TX16S / TX16SMK2, Radiomaster Zorro

Operating System (OS)

macOS

OS Version

No response

Anything else?

No response

elecpower commented 1 year ago

Feature should have been disabled, until fixed, when move to yaml for all radios. Has never worked (OTX days) when Horis came out.

pfeerick commented 1 year ago

Indeed... like in https://github.com/EdgeTX/edgetx/issues/3676

elecpower commented 1 year ago

The backup and restore, in its current form, was implemented to backup and restore eeproms thus why disabled for Horis. I propose the feature be disabled for all radios until it is revamped in say 2.10. The feature should backup the firmware, RADIO, MODELS and optionally other parts of SD card.

philmoz commented 1 year ago

I'm taking about the read / write model settings functions - the ones used to create .etx files in companion and save them back to the radio as YAML files. Screenshot 2023-07-04 at 4 05 48 pm

All Companion versions will destroy the 2.10 RADIO.YML file read from the radio - which also means you can't run the simulator from setting loaded from a real radio.

elecpower commented 1 year ago

Oops my bad I read the issue title and was blinkered reading what followed.

elecpower commented 1 year ago

@philmoz there has been so much going on but I thought you retrograded the YAML so it was 2.8.4 and 2.10 compliant or was that read only?

elecpower commented 1 year ago

'read only' on reading but write 2.10?

philmoz commented 1 year ago

The previous fixes were for the wrong names being written for pots/sliders to radio.yml and to revert the custom function switch names for compatibility.

This one is more extensive and complex. I looked at reverting the names in the firmware code; but it's not trivial. Previously pots and sliders were saved to separate sections in the radio YAML file, now they are merged into one (with different names).

philmoz commented 1 year ago

I don't know whether it would be easier to fix Companion or revert the names in the firmware code :(

elecpower commented 1 year ago

Whatever is done is going to be a lot of work and likely to have to be done twice :-( I fear that until Companion is refactored to use the json files we will be fighting an uphill battle. Without checking I suspect the dynamic adc mappings are going to break radio to radio conversions too as so much is positional or hard coded. Would you mind uploading working 2.8.4 and 2.10 radio settings then I can see if there are any practical gymnastics such as I had to do for fixing center beep and warnings.

philmoz commented 1 year ago

radio2.9.yml.txt radio2.10.yml.txt

TX16S radio yaml files from 2.9 and same file after updating to 2.10.

The 'calib', 'potsConfig' and 'slidersConfig' sections are the current concern.

elecpower commented 1 year ago

Thanks Phil. I'll ponder over a very strong coffee later tonight...

philmoz commented 1 year ago

I think the 'calib' section can be reverted by writing the old names.

The pots/sliders looks problematic because the underlying data structure has changed.

elecpower commented 1 year ago

Hi Phil, after a strong brew, I feel it is practical for Companion to handle old and new yaml formats. Based on semver use either the old or new label to cpn indexes for decoding. Likely need to process the new potsconfig twice ie once for pots and once for sliders. Add a new encode lookup to map the cpn indexes to new labels. Plus add the offset gymnastics like libsim fixes. Don't encode sliderconfig section.

Question: when the firmware reads the radio.yml file does it expect the analogs to be in order ie same as json file? eg would P1, P2, SL1, SL2, P3, P4 break the firmware?

philmoz commented 1 year ago

Question: when the firmware reads the radio.yml file does it expect the analogs to be in order ie same as json file? eg would P1, P2, SL1, SL2, P3, P4 break the firmware?

I tried manually editing the radio.yml file on my TX16S and changing the order in the 'potsConfig' section. On restart I got 'bad radio data' error, and it went into the sd card setup process, wiping the radio.yml file and the selected modelX.yml file.

So I would say, yes, the firmware expects a certain order.

philmoz commented 1 year ago

An interim fix might be to change Companion to be able to read the new YAML format; but still write the old version. The firmware can handle both.

pfeerick commented 1 year ago

I tried manually editing the radio.yml file on my TX16S and changing the order in the 'potsConfig' section. On restart I got 'bad radio data' error, and it went into the sd card setup process, wiping the radio.yml file and the selected modelX.yml file.

Just double-checking... you did remember to update the "manuallyEdited" flag to "1" when making that change? I was able to reorder the entries in the potsConfig block without any settings loss, and the firmware put it back in the right order on its own.

An even simpler, and more permant solution we should probably put in place (and could do for both 2.9.0 and 2.8.5) is to actually check the semver flag, and if it is newer than the version of Companion being run, throw up a warning saying something like "Your configuration appears to be from a newer version of the radio firmware than this version of Companion expects. Model and radio configuration may be corrupted if you continue." with the option to continue (or even a checkbox "I know the risks"?), or abort. I say a warning rather than a hard block, as it should be up to the user if they "need" continue or not - i.e. 2.8.5 isn't going to mangle 2.9 settings, but 2.10 would mangle 2.9 settings.

philmoz commented 1 year ago

Just double-checking... you did remember to update the "manuallyEdited" flag to "1"

Thanks, that works.

Of course now my TX16S decided to fail (all the keys and trim switches stopped working) so I'm up the proverbial.

elecpower commented 1 year ago

Re semver check, I was already adding to 2.10 but as you say wouldn't hurt to add to 2.8.5 and 2.9 RC too. I'll knock up a PR from which you can cherry pick the commit into 2.8.5 and 2.9.

elecpower commented 1 year ago

Is this acceptable?

warnmsg

gagarinlg commented 1 year ago

I like it

pfeerick commented 1 year ago

Yup, that'll do. :) We can always fine-tune the wording later if needed. As long as you're told if you continue and it breaks, there's no warranty, I'm happy. 😆

elecpower commented 1 year ago

That's the polite wording ;-)

ror1948 commented 1 year ago

so is companion 2.10 ok with nightly builds?

pfeerick commented 1 year ago

It should be mostly ok now.