Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(cpn): Support JSON radio hardware definitions #4406

Merged
merged 72 commits into from
Feb 4, 2024

Conversation

elecpower
Copy link
Collaborator

@elecpower elecpower commented Dec 11, 2023

Summary of changes:

  • remove most of the 1st attempt
  • move from static board configurations to use radio definitions json files. This also has the advantage of keeping radio sims, firmware and Companion in sync
  • group separate structs for same item eg pots into single structs
  • maintain support for reading legacy radio settings configuration formats eg bin, otx, etx and convert all formats inc yml as necessary
  • yaml encode and decode
  • IMPORTANT: 'cmake --build . --target native' and 'cmake --build native --target hardware_defs' MUST be executed for each radio before 'cmake --build . --target native' and 'cmake --build native --target companion'. The companion and simulator targets only includes the files for the pattern '<build-output>/native/radio/src/*.json'. The path will change to something like '<source directory>/radio/src/hwdefs/*.json' when the files are not dynamically generated.

Fixes #3592
Fixes #3982
Fixes #4099
Fixes #4507
Fixes #4589

TODO

  • generate hw defn json files on each compile of Companion and remove point in time copies from repo
  • issue: extra pots configured as multi pos pot do not respond to gui clicks eg TX16S. Update: appears to be working again.
  • gyros ie TILT_X and TILT_Y are not included in json files but need to be in source lists. Should they or if compiler directive then should be profile firmware option rather than hardcoded. Update: hardcode their inclusion until included in json file.
  • mouse/joysticks a couple of json files include but most do not so similar to gyros. Update: hardcode their inclusion until included in json file.
  • decision: board to board conversion where in past switches were remapped automatically. Update I decided that only a few (mainly FrSky) radios had this implemented, to map based on tags and thus auto re-index.
  • clean up code inc removing debug messages, unused code and consolidate
  • bucket loads of testing including loading all supported formats, 2.10 and prior yml versions and the usual to and from 2.10 radios
  • issue: radio libsim (used TX16S) flex switch position not reflected in Radio Settings Analogs or Switches Test page. Update: convert switch position to analog value and pass value to flex analog.
  • issue: function switches needs further work especially radio libsim eg T20. Update:: do not create std switches
  • issue: some json files eg x9d+ do not have default type for flex inputs thus they are initialised to None which is an annoyance for pilots. Unable to determine default for pots and sliders across all radio types. Action: Fix json files or less favourable resort to hardcoding! Update: radio/utils/hw_defs/pot_config.py had refs to x9dp and x9dp2019 when should be x9d+ and x9d+2019. Fixed with fix(x9d+): Default pots and sliders config #4489
  • treat Function Switches as sub-type of general switches like Flex Switches

TODO Post Merge

  • update build instruction documentation for all toolchains
  • update scripts for all toolchains
  • create list of additional configuration to be included in json files to avoid hardcoding in Companion
  • when conversion from older file formats is withdrawn refactor Board namespace to class (too much work for this PR), add json firmware config similar to radio hw config and refactor Firmware (consolidating and simplifying).

@pfeerick
Copy link
Member

I'm going to post stuff as I encounter issues, rather than summarise or lump stuff together. I must be one unlucky chump... right out of the bat I hit this (testing will on Windows unless otherwise stated):

With TX16S profile selected I get this message no less than three times. Tried with a new profile, same result

image

@pfeerick
Copy link
Member

pfeerick commented Jan 15, 2024

Adding a new Profile

image

Error shows once on selecting Radiomaster Pocket as the Radio Type, and then two more times upon clicking Ok to leave the screen.

Doesn't happen with the Boxer, TPro or X12S profiles I had configured, so I am thinking it's related to the recently added radios.

@pfeerick
Copy link
Member

pfeerick commented Jan 15, 2024

Reading the RM Pocket (is running nightly from a few days ago):

Read models OK, upon checking a configured model, inputs had no sources, trim set to no trim (unconfigured so ON in the model). A mixer line with a source was also missing it's source.

For reference, attached is the model in question, as it was on the radio, and what Companion exported after reading from the radio.

model_from_radio.zip
model_from_cpn.zip

Reading a 2.9 radio and firmware (Jumper T-Lite) seems ok. I've not written as I don't have a backup yet 😆
Reading a 2.9 radio with 2.10 firmware (FrSKY X9D+) again has no sources.

image

Reading from a 2.9 radio and firmware (RM TX12MK2), writing to it, and switching to 2.10 firmware results in trashed / NONE input sources.

@elecpower
Copy link
Collaborator Author

@pfeerick unable to load [radio].json 99.9% of the time means the json files were not generated prior to Companion being compiled (see my IMPORTANT NOTE above)

@elecpower
Copy link
Collaborator Author

If there is no json file then all bets are off about what happens subsequently...

@elecpower
Copy link
Collaborator Author

@pfeerick building a libsim will generate its json file as a necessary step

@elecpower
Copy link
Collaborator Author

elecpower commented Jan 15, 2024

@pfeerick so I downloaded the Appimage from the last commit and it is missing some radios. Will have to scan the build step to see if I can see why. I know the tx16s works as that is one is tested extensively.
Update: nothing obvious in the logs so appears a build environment issue. Since the environment is destroyed I will add some debug messages to the cmake hwdef resource build step.

@3djc
Copy link
Collaborator

3djc commented Jan 15, 2024

I guess the issue might come from recent radio that don't have simu defined (waiting for an automatically generated one). We can define a generic one in the meantime

@elecpower
Copy link
Collaborator Author

Looks like one of my clean up/fix-its broke the yaml parser and it was working just fine. Will look at tomorrow. By then the build will have finished so can check the debug messages.

@elecpower
Copy link
Collaborator Author

I'm going to post stuff as I encounter issues, rather than summarise or lump stuff together. I must be one unlucky chump... right out of the bat I hit this (testing will on Windows unless otherwise stated):

With TX16S profile selected I get this message no less than three times. Tried with a new profile, same result

image

Yeah just one of those things in the way Boards are handled in the older code and why it needs to be refactored.

@elecpower
Copy link
Collaborator Author

Reading a 2.9 radio with 2.10 firmware (FrSKY X9D+) again has no sources.

Please clarify, as if the firmware is 2.10 then the radio s 2.10 not 2.9 or am I missing something?

@elecpower
Copy link
Collaborator Author

Fall on sword moment re yaml :-(
If time hopefully fix tonight.

@elecpower
Copy link
Collaborator Author

Adding a new Profile

image

Error shows once on selecting Radiomaster Pocket as the Radio Type, and then two more times upon clicking Ok to leave the screen.

Doesn't happen with the Boxer, TPro or X12S profiles I had configured, so I am thinking it's related to the recently added radios.

No libsim built for Pocket so that is why it is missing

@pfeerick
Copy link
Member

pfeerick commented Jan 15, 2024 via email

@elecpower
Copy link
Collaborator Author

Adding a new Profile

image

Error shows once on selecting Radiomaster Pocket as the Radio Type, and then two more times upon clicking Ok to leave the screen.

Doesn't happen with the Boxer, TPro or X12S profiles I had configured, so I am thinking it's related to the recently added radios.

So the Build logs show that the TX16S is the last libsim compiled. Very strangely, to me at least, is that the hwdef resource file is being constructed every time before a libsim is compiled and not just once before Companion is compiled as was the plan. This explains why the last compiled libsim in this case TX16S is not added to the Companion hwdef resource file.

A CMake guru to the rescue please!

@pfeerick
Copy link
Member

pfeerick commented Jan 16, 2024

Ouch... so it looks like each time cmake --build . --target native-configure is called by the loop in tools/build-companion.sh that builds each simulator plugin/lib, your hardware definitions test is being run (hence why the tx16s is missing as the last one run), and the results of each successive pass cached.

This should probably have been restricted to the package / installer targets? Since that is what tools/build-companion.sh uses to actually do the companion build? In the meantime... I'm going to make one possibly naive change to the build script, and see if I can trip it for the companion configure pass... lets see what github actions says! 🤪

@elecpower elecpower force-pushed the elecpower/cpn-adc-refactor-v2 branch from 301ee07 to fd21d05 Compare February 1, 2024 04:06
@pfeerick
Copy link
Member

pfeerick commented Feb 4, 2024

I've tried several combinations of TX16 and Zorro, pulling configs from 2.9 to 2.10 using Companion, and then reading and writing them back, and nothing obvious is jumping out, so I think you banished most of the gremlins ;)

Unless there is something outstanding I think its time to merge this, and get more people using it. Then we can also sort out getting at least minimal functionality for the Pocket, etc. (enabling simulator).

@elecpower
Copy link
Collaborator Author

I have no other tweaks I wanted to sneak in so let the beast run free

@pfeerick pfeerick changed the title Companion refactor to support json radio hardware definitions refactor(cpn): Support JSON radio hardware definitions Feb 4, 2024
@pfeerick pfeerick merged commit 8329461 into main Feb 4, 2024
46 checks passed
@pfeerick pfeerick deleted the elecpower/cpn-adc-refactor-v2 branch February 4, 2024 05:59
@pfeerick
Copy link
Member

pfeerick commented Mar 18, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment