Companion firmware simulator crashes on loading or switching to model with internal MPM selected

[mha1]

Is there an existing issue for this problem?

• I have searched the existing issues

What part of EdgeTX is the focus of this bug?

Companion

Current Behavior

Windows64 standalone firmware simulator crashes on starting up or switching to a model with internal MPM selected. This is the last Debug Output messages:

Expected Behavior

Don't crash

Steps To Reproduce

install Companion W64 version
run Firmware Simulator 2.8
select model with internal model MPM enabled

Version

2.8.2

Transmitter

Radiomaster TX16S / TX16SMK2

Anything else?

No response

[mha1]

2.9 crashes also. Building Companion without #3369 resolves the problem (but of course unfixes the original problem it was fixing)

[pfeerick]

Hm, maybe we forgot to update something in companion as part of that pr. It'll probably be a few hours before I'll be in any fit state to look at this, but I'll try to look at it soon. Thanks for narrowing it down.

[pfeerick]

Hm... this gets weirder and weirder... it breaks on windows simulator, but not on Linux builds...

[mha1]

Don't know why Linux cpn is not crashing but anyhow it should produce questionable results. I think cpn is pretty outdated in this area as some cpn data structures need to reflect radio structures. The major pain point is the fact data structures are defined in both radio and cpn and even worse the data definitions are not identical. check out cpn's const Multiprotocols multiProtocols vs it's radio gui_common.cpp counterpart. Not even string definitions are shared, see QString Multiprotocols::protocolToString. And there is code in cpn's yaml de/encode to adjust for the FrSky mess. This is probably also outdated depending on how far radio code got cleaned up, see cpn's static void convertMultiProtocolToEtx and void convertEtxProtocolToMulti in yaml_moduledata.cpp. And maybe if (pdef.hasFailsafe || (module.multi.rfProtocol == MODULE_SUBTYPE_MULTI_FRSKY && (module.subType == 0 || module.subType == 2 || module.subType > 3 ))) in companion\src\modeledit\setup.cpp is dodgy too.

[pfeerick]

True. Shared code is something that has already been identified as needed. But more to the point here, I think is we need to identify why it is the radio simulator that is crashing on *protocol scanning*... i.e. I don't think this actually has anything to do with companion - standalone simulator is crashing 😉

…

On Sun, 2 Apr 2023, 7:49 pm Michael, ***@***.***> wrote: Don't know why Linux cpn is not crashing but anyhow it should produce questionable results. I think cpn is pretty outdated in this area as some cpn data structures need to reflect radio structures. The major pain point is the fact data structures are defined in both radio and cpn and even worse the data definitions are not identical. check out cpn's const Multiprotocols multiProtocols vs it's radio gui_common.cpp counterpart. Not even string definitions are shared, see QString Multiprotocols::protocolToString. And there is code in cpn's yaml de/encode to adjust for the FrSky mess. This is probably also outdated depending on how far radio code was clean up, see cpn's static void convertMultiProtocolToEtx and void convertEtxProtocolToMulti in yaml_moduledata.cpp. And maybe if (pdef.hasFailsafe || (module.multi.rfProtocol == MODULE_SUBTYPE_MULTI_FRSKY && (module.subType == 0 || module.subType == 2 || module.subType > 3 ))) in companion\src\modeledit\setup.cpp is dodgy too. — Reply to this email directly, view it on GitHub <#3423 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABJ66KLPJ775LJUQKVZGFELW7FDQZANCNFSM6AAAAAAWPU2MXY> . You are receiving this because you commented.Message ID: ***@***.***>

[philmoz]

My guess would be it's the change to line 1259 in radio/src/gui/gui_common.cpp
{MODULE_SUBTYPE_MULTI_FRSKY_R9, 7, true, false, STR_SUBTYPE_FRSKYR9, nullptr},

The second value was 6; but was changed to 7. The value should be 6 (maxSubType, not subType count).
The code in fillSubProtoList is falling off the end of the array.

[mha1]

You are absolutely right. Those values are not the number of entries but the max index (entries-1). It works. Nevertheless for Companion (not the simulator) to show the correct protocol and subtype entries it is required to use the same protocol and subtype definitions and strings.