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.63k stars 345 forks source link

Companion 2.6 transmitter to transmitter conversion bug #1413

Open Nerenye opened 2 years ago

Nerenye commented 2 years ago

Companion 2.6 cb7915f680fbd307bae65c12d7c1d15a947121d2

Scenario:

ETX Model/Radio Backup File from Horus X12S Companion 2.6 witch Profile of Radiomaster TX12 loaded

Select Open and import the Backup of the Horus.

You get a conversion Notice but no list of Converted entrys

If you inspect a model , every line which would have needed a conversion is now corrupted and the Value blank

Broken: See L01 "V1" , L02 "And Switch"and L04 "V1" image

Correct: image

Same behavior is happening if you open a Companion with Horus X12S profile copy the model you want to convert and paste it into a different companion Instance with TX12 Profile active

if you start the simulator at this point it seems like the radio profile of the Horus is loaded into the TX12 since the switches are configured like the Horus.

Broken Radio Config launched from X12S Model file image

Correct Radio Config launched with blank TX12 Model file image

You can convert the files Correctly though. If you load Companion with the initial Radio Profile active eg Horus in my case, Open the Backup and then switch to the different Radio... You get the conversion warning and afterwards the list of all converted lines. This way the Conversion is working fine (SH converted to SD) for example and also Simulator is working fine.

in Case, the Problem is within my backup ill add it here ElectraMKJ.etx.txt

raphaelcoeffic commented 2 years ago

To be truthful, this has not been implemented in YAML, as it was not necessary to achieve proper loading. The mixup here is potentially not offering taking over the radio configuration from the selected profile rather than using the current one.

Nerenye commented 2 years ago

so it´s a coincident that it works fine if you load the files with correct Profile and than switch?

raphaelcoeffic commented 2 years ago

so it´s a coincident that it works fine if you load the files with correct Profile and than switch?

This is a different feature, let's say. I'd need to re-check with @elecpower to know how that really works.

koaxheli commented 2 years ago

Radiomaster TX16S

Also still found in "Dauntless" v2.5.0 still OK. Spe_Func_OK

In "Santa" v2.6.0-RC1 lines moved. Spe_Func_nichtOK

In addition the Fuctionen in yml. Spe_Func_YML

koaxheli commented 2 years ago

Addendum In addition the Fuctionen in yml Spe_Fun_YML .

elecpower commented 2 years ago

As @raphaelcoeffic assumed when reading yaml for a board that does not match the profile the radio hardware settings are not being initialised correctly (see below). Consequently as the settings are read for each model they are checked against these dodgy radio hardware settings. When there is an issue with a model setting it is set to NONE for safety.

Screenshot from 2022-01-13 06-18-48

elecpower commented 2 years ago

When changing profiles after any radio and model settings file has been loaded a different conversion process occurs that is common across yml and bin formats. Reading bin formats goes through a very convoluted and granular process that also reports what was changed. A totally different reading and interpretation process is applied to yml files. Copy and pasting across companion instances using different profiles is fraught with danger as it uses a memory copy via the clipboard with little to no checking as the multiple instances are not aware of each other.

Nerenye commented 2 years ago

I just tried any possible combination in order to reproduce the issue. i usually would not work with two instances! But Loading different Model Backups can occour quite often as i do have 2 different radios and i´m maintaining some models for a friend of mine with a third.

So we could either inform the user that a not matching file is opened and he/she should open it in the correct profile first and switch over afterwards. Or the logic needs to be implemented further down the road?

elecpower commented 2 years ago

The user gets a warning and has the option to back out

Screenshot from 2022-01-13 06-55-07

Nerenye commented 2 years ago

Yeah but there is no conversion. The popup is wrong ?!? It is just destroing the file. Wether it works absolutely fine when switching the profile from the correct radio profile. Where you get the same warning but a conversion list and a working model/radio profile afterwards.

elecpower commented 2 years ago

Also it is not just the radio hardware that could be wrong, any radio settings could be incompatible and cause some unexpected consequences in companion and when written back to the radio!

elecpower commented 2 years ago

An option is to ignore the radio.yml file and set the values to the profile defaults. This needs some thought.

Nerenye commented 2 years ago

yeah i get that. thats why i think it should either be mentioned ,or better the correct way should be mentioned or disabled all together. Don´t get me wrong! I get that these are different code paths but one works and the other does not but doesn´t tell you that. in my case i wasn´t been able to arm my motor and realised therefore that there is something wrong. but it could be worse. Without notification about converted lines, you usually think everything went fine.

elecpower commented 2 years ago

Even using the default profile would still have possible model settings that cannot convert eg where a switch is not supported by the profile and thus be set to NONE.

Nerenye commented 2 years ago

how could that be? when creating a new tx profile you get the default values of the switches . right? if you switch profiles after loading to the correct profile, it converts even non existing switches to similiar ones on the new tx. Like SH on the Horus to SD on TX12. Sure if someone modded in a new switch that does not exist on the standard radio, you can´t account for that but for standard radios that should be possible and is working in 2.6 if radio profile is switched after loading the files

raphaelcoeffic commented 2 years ago

@koaxheli your issue has already been fixed here: https://github.com/EdgeTX/edgetx/pull/1367