Could not save for pre-release multiprotocol

[rdba2k]

Is there an existing issue for this problem?

• I have searched the existing issues

What part of EdgeTX is the focus of this bug?

Transmitter firmware

Current Behavior

I use pre-release multiprotocol, for example,FX/9630. I can change the setting for external module in model A, then I switch to model B, then switch back to model A, the configuration in model A is lost. If I use FX/860, the configuration is kept. Edgetx 2.8.5 seems no problem.

Expected Behavior

Maybe edgetx 2.9 checks the multiprotocol and invalidate the protocol not defined in its header files, which prevents us to use latest multiprotocol version than the release version when 2.9 is built.

Steps To Reproduce

Described in current behavior section

Version

2.9.0

Transmitter

Radiomaster TX16S / TX16SMK2

Operating System (OS)

No response

OS Version

No response

Anything else?

No response

[pfeerick]

It should be using the protocols list as reported by the module, and only falling back to the pre-defined list if the module is not present or not communicating. @mha1 Any ideas?

[mha1]

edgetx/radio/src/MultiSubtypeDefs.h

Line 221 in e807702

{MODULE_SUBTYPE_MULTI_FX, 1, false, false, STR_SUBTYPE_FX, nullptr},

change this line to read "2," instead of "1,"

[3djc]

edgetx/radio/src/MultiSubtypeDefs.h

Line 221 in e807702

{MODULE_SUBTYPE_MULTI_FX, 1, false, false, STR_SUBTYPE_FX, nullptr},

change this line to read "2," instead of "1,"

While this hides the issue, it is likely not the right fix. The radio should get updated list from the module and handle it gracefully without code change

[mha1]

those definitions must match multi.txt otherwise yaml can't decode the saved subprotocol when loading the modelxx.yml

[3djc]

Then that YAML part is wrong and needs to be fixed. The multi needs to be able to list new protocols independently of edgetx code

[raphaelcoeffic]

those definitions must match multi.txt otherwise yaml can't decode the saved subprotocol when loading the modelxx.yml

Since when? AFAIK, only proto numbers are saved.

[mha1]

moduleData: 
   0:
      type: TYPE_MULTIMODULE
      subType: 57,0
      channelsStart: 0
      channelsCount: 16

subtype holds protocol and subprotocol. In this case 57 -> protocol HoTT, 0 -> subprotocol SYNC

I shouldn't answer reading stuff on my phone and answer on the phone. Question number 1 needs to be which MPM firmware was used and does this firmware send the added third subprotocol. If not you fall back to the internal definitions and in this case the definitions need to match, i.e. lines 133 (defines 3 sub protocols) and 221 (also needs to state we have 3 subprotocols - counting starts from zero hence 1 -> 2).

[mha1]

@rdba2k just out of curiosity: after switching back to model A you see Mode: MULTI FX and RF Protocol: in the External RF setup page, correct? Have you tried connecting the receiver? It should still work as if 9630 was selected. Does it?

Can you please attach model yaml file of model A after switching from B back to A. And please let us know which MPM firmware exactly you are running.

[mha1]

I can confirm the reported problem. I tried this with 2.9.1 and the latest MPM merge build (1.3.3.32). EdgeTX scans the MPM and the MPM correctly sends the subProto list for protocol FX. FX/9630 can be selected (no need for #4145). Protocol data is stored in model.yaml as expected (58->FX, 2->9630):

moduleData: 
   0:
      type: TYPE_MULTIMODULE
      subType: 58,2
      channelsStart: 0
      channelsCount: 16
      failsafeMode: NOT_SET

Then shutting down the radio and starting it up again clears the models module data:
moduleData:

   0:
      type: TYPE_NONE
      subType: 0
      channelsStart: 0
      channelsCount: 16
      failsafeMode: NOT_SET 

Need to find out why.

[mha1]

Building 2.9.1 with the MultiSubtypeDefs.h lines 133 and 221 changed to add the 9630 subprotocol works as expected. No more clearing of the module's data. But I agree with @3djc this is only masking the real problem. I think there is an MPM protocol scan trigger missing or going wrong. Will dig deeper later.

[mha1]

Well, it's like I suspected earlier. It's yaml kicking it out ( r_modSubtype()) because the model yaml data is read before the module is initialized. Only at initialization of the module in multiInit() a scan is triggered. Until multiInit() did its job yaml decoding of subtype information relies on the internal MPM definitions.

115.94s: YAML model reader
115.95s: Multi protocol sub-type 2 exceeds number of supported sub-types. Module set to: OFF

And yaml will fail the subtype as the internal definitions don't know about the additional subtype

Haven't gone deeper so just thinking loud: this is kinda tricky because we'd have to squeeze in a scan into the yaml decoding. Right after we know if the module type is an MPM and if its internal or external but before the yaml subtype decoding is done.

I think it's much easier to keep internal definitions in sync with multi.txt. We have to do this in any case for editing models without live MPM and Companion. So my proposal to solve this is the above change to line 221 in PR #4145

Other ideas?

[3djc]

The change to multi protocol tables is needed, but clearly not enough, as you don't want to force users to a nightly because of an MPM change. The yaml loader needs to allow protocols it doesn't know about (and since it is only numbers stored, there should not be anything preventing that)

[rdba2k]

After switching back to model A, it is empty in external module configuration, as shown from GUI. My MPM version is 1.3.3.31. My situation is more complex, it is eachine tx16s. Due to its quality problem, internal mpm does not work with some protocols such as v761, fx etc, so I have an external mpm. Should I copy the file Multi.txt from multi source code to sdcard /scripts/tools to fix the issue? I copied all files under Lua_scripts in multi git to /scripts/tools, it looks like the new file name is multiChan.txt.

https://github.com/pascallanger/DIY-Multiprotocol-TX-Module/tree/master/Lua_scripts

[rdba2k]

Multi.txt is a different file in multi git
https://github.com/pascallanger/DIY-Multiprotocol-TX-Module/blob/master/Multiprotocol/Multi.txt

[raphaelcoeffic]

The point is, this code should go away! (coming from #3435)

      if(type > MODULE_SUBTYPE_MULTI_LAST) {
        TRACE("Multi protocol: %d exceeds supported protocols. Module set to: OFF", type);
        md->type = MODULE_TYPE_NONE;
      } else {
          if(subtype > getMultiProtocolDefinition(type)->maxSubtype) {
            TRACE("Multi protocol sub-type %d exceeds number of supported sub-types. Module set to: OFF", subtype);
            md->type = MODULE_TYPE_NONE;
          } else {
            md->multi.rfProtocol = type;
            md->subType = subtype;
          }
      }

And we should make sure that the UI is able to cope with some types / sub-types not being in any definitions.

We should never rely on MPM definitions being either available or loaded in YAML.

[rdba2k]

It looks to me the MPM protocol scanning works correctly. Is it possible adding a scan button in radio GUI when "multi" is chosen. When scan button is pressed, a yml file containing the mappings between protocol/subprotocol numbers and names are created in the models directory. It also includes MPM version number. The companion could use this yml file to display MPM version and both protocol numbers and names. The sdcard content could come with some default yml for 1.3.3.20 MPM. The files could be for internal MPM and external MPM individually. Thanks.

[raphaelcoeffic]

@rdba2k this is totally unneeded! Scanning is automatic as soon as the module is turned ON. In the YAML code, we should use only the protocol and sub-type numbers, as the string mapping can only be known at runtime when the module runs. It would be much too complex to operate with list files just for this purpose.

Please test with #4146, it should allow you to handle protocols that are not in the static definitions as we used to in 2.8.x.

[pfeerick]

I've not tried the PR, but my expectation would simply be that the input be validated as sensible (i.e. a number), and then left alone if EdgeTX doesn't recognise the number and map it to a name (allowing for future MPM releases + no module present/responding( to not be an issue. Which I believe is the case with this PR

…

On Mon, 2 Oct 2023, 3:41 am Raphael Coeffic, ***@***.***> wrote: @rdba2k <https://github.com/rdba2k> this is totally unneeded! Scanning is automatic as soon as the module is turned ON. In the YAML code, we should use only the protocol and sub-type numbers, as the string mapping can only be known at runtime when the module runs. It would be much too complex to operate with list files just for this purpose. Please test with #4146 <#4146>, it should allow you to handle protocols that are not in the static definitions as we used to in 2.8.x. — Reply to this email directly, view it on GitHub <#4144 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABJ66KJNNVI7YU3ZZVVMT5LX5GTNVANCNFSM6AAAAAA5OAQYLA> . You are receiving this because you commented.Message ID: ***@***.***>