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.57k stars 333 forks source link

Building RM T8 firmware with option -DRADIOMASTER_RTF_RELEASE=YES fails #3480

Closed mha1 closed 1 year ago

mha1 commented 1 year ago

Is there an existing issue for this problem?

What part of EdgeTX is the focus of this bug?

Transmitter firmware

Current Behavior

Building T8 firmware with option -DRADIOMASTER_RTF_RELEASE=YES fails with errors in radio/src/targets/taranis/bind_button_driver.cpp: In function 'bool setBindProtocolSelection()'.

Expected Behavior

If BIND_KEY is still a valid Option for T8 build should complete otherwise BIND_KEY should be removed as build option

Steps To Reproduce

MSYS build current main with cmake line cmake -G "MSYS Makefiles" -Wno-dev -DCMAKE_PREFIX_PATH=$HOME/5.12.9/mingw73_64 -DSDL_LIBRARY_PATH=/mingw64/bin/ -DPCB=X7 -DPCBREV=T8 -DRADIOMASTER_RTF_RELEASE=YES -DCMAKE_BUILD_TYPE=Release ../

Version

Nightly (Please give date/commit below)

Transmitter

Radiomaster T8

Operating System (OS)

No response

OS Version

No response

Anything else?

No response

pfeerick commented 1 year ago

The error in question being:

/src/radio/src/model_init.cpp: In function 'void setVendorSpecificModelDefaults(uint8_t)':
/src/radio/src/model_init.cpp:109:39: error: 'struct ModuleData' has no member named 'setMultiProtocol'
  109 |   g_model.moduleData[INTERNAL_MODULE].setMultiProtocol(MODULE_SUBTYPE_MULTI_FRSKY);
      |                                       ^~~~~~~~~~~~~~~~
/src/radio/src/model_init.cpp:109:56: error: 'MODULE_SUBTYPE_MULTI_FRSKY' was not declared in this scope; did you mean 'MODULE_SUBTYPE_MULTI_FRSKYL'?
  109 |   g_model.moduleData[INTERNAL_MODULE].setMultiProtocol(MODULE_SUBTYPE_MULTI_FRSKY);
      |                                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                        MODULE_SUBTYPE_MULTI_FRSKYL
/src/radio/src/model_init.cpp:110:49: error: 'MM_RF_FRSKY_SUBTYPE_D8' was not declared in this scope
  110 |   g_model.moduleData[INTERNAL_MODULE].subType = MM_RF_FRSKY_SUBTYPE_D8;
      |                                                 ^~~~~~~~~~~~~~~~~~~~~~

I believe this code to be specific to the T8 and T8 alone, which is normally only built via tools/build-radiomaster.py at the present time, hence it wasn't updated as part of the frsky cleanup PR.

mha1 commented 1 year ago

It misses opentx.h and some changed defines too. As it is also affected by the Multi PR I'll fix it as part of the Multi PR if you agree.

pfeerick commented 1 year ago

If you don't mind, since it's semi-related. If it won't co-operate don't worry about it, can hassle JC since he might have a handset. 😁

mha1 commented 1 year ago

I have updated PR https://github.com/EdgeTX/edgetx/pull/3435 to fix this issue. Can you ask JC to build T8 firmware using https://github.com/EdgeTX/edgetx/pull/3435 with -DPCB=X7 -DPCBREV=T8 -DBIND_KEY=YES and give it try?

changes:

mha1 commented 1 year ago

Bind button should bind: Center: FRSKY D D8 Top left: FRSKY X D16 FCC Top right: FRSKY X D16 LBT Bottom left: FRSKX X2 V2.1 D16 FCC Bottom right: FRSKY X2 V2.1 D16 LBT

mha1 commented 1 year ago

Here's t8 firmware for testing: fw-t8-pr3435.zip

3djc commented 1 year ago

The way it was designed was not to have BIND_KEY as an option, but flow from RADIOMASTER_RTF_RELEASE, as RADIOMASTER_RTF_RELEASE disables edition and is designed to run headless (you have to skip any warning and such), or you don't define it and you need screen and it behaves like a normal radio.

But this firmware requires a screen at first bot to create by default the yaml file without user intervention. Also, multi is not enabled by default, so it would not work also because of that

mha1 commented 1 year ago

Thanks for your feedback. So if RTF is defined bind key should be available otherwise no bind key? Is RTF still a thing?

gagarinlg commented 1 year ago

This was only used for a T8, non lite and non pro? The Lite does not have OpenTX and the Pro comes with a display.

mha1 commented 1 year ago

at least the build scripts list option BIND_KEY for T8 only

3djc commented 1 year ago

Thanks for your feedback. So if RTF is defined bind key should be available otherwise no bind key? Is RTF still a thing?

Exactly, but RTF is broken beyond bind key only, because some warning remain, that you cannot see or acknowledge without a screen (RTF is screen less, but both 'lite' and 'pro' run otx or edge, just a version without warning and using bind key on lite)

mha1 commented 1 year ago

@3djc ok, gotcha. Well, this PR certainly did not try to fix RADIOMASTER_RTF_RELEASE in total. This addresses just the bind key driver which was left behind in some previous FrSky code cleanup. If RADIOMASTER_RTF_RELEASE is broken beyond the bind key driver I'd say it should be (temporarily) disabled. Did headless T8's materialize at all? Just curious what they look like.

3djc commented 1 year ago

I'll fix the rest once this PR is merged

mha1 commented 1 year ago

@3djc Great. Do you have hardware to test this? I can attach a proper T8 RADIOMASTER_RTF_RELEASE build for you to check out this PR's changes.

3djc commented 1 year ago

Yes I have a pro and a lite, but don't worry too much, tested ok on a pro, will test on a lite when merge and fix what needs fixing

mha1 commented 1 year ago

I'll fix the rest once this PR is merged

@3djc it's in main now