Mcu ID for vtx tables

[kristjanbjarni]

Reads craft name from MSP and uses that name to store the vtx tables. If the craft name is empty it falls back on using the transmitter model name as before. This should solve the problem of using an incorrect vtx table when people have many different crafts with different vtx hardware under the same model name.

[kristjanbjarni]

Addressed all of the review comments by removing dead code and adding check for conditional download of craft name if vtx tables are not supported.

[klutvott123]

@kristjanbjarni Looks good 👍. Tested and works as expected. Could you squash please?

[klutvott123]

There is still the issue that craft name isn't guaranteed to be unique just like model name. I think the best solution would be if each FC had a unique identifier. STM devices have a unique 96 bit id and it's available through MSP(MSP_UID). This could possibly simplify things and remove a lot of confusion.

@mikeller Any thoughts on using the mcu id for this?

[kristjanbjarni]

@klutvott123 This is my first pull request so I am not really sure how I squash correctly, seems I messed it up?

There is still the issue that craft name isn't guaranteed to be unique just like model name. I think the best solution would be if each FC had a unique identifier. STM devices have a unique 96 bit id and it's available through MSP(MSP_UID). This could possibly simplify things and remove a lot of confusion.

I think the only reason for craft name not being unique is if somebody is using it for their pilot name. Since there already exists "Display name" for pilot name I would consider that to be erroneous usage of craft name. But maybe mcu id would be a better idea.

[mikeller]

@klutvott123: Good idea! Using the MCU id will solve this once and for all.

[klutvott123]

All right let's do it!

@kristjanbjarni I agree, but there will always be someone doing it wrong. By using the mcu id it should just work without the user having to keep track of all their model names and craft names. The code you have already written could easily be adapted to get the mcu id from the FC. Would you like to have a go at it?

You had two commits on your branch and wanted to squash them into one commit.
You would do this with git rebase -i HEAD~2. This brings up a text editor with a list of the two last commits prepended with pickalong with some instructions. Change pick to squash(but fixup might be better in this case) for the second commit in the list, save and exit. Now you should have only one commit on your local branch. Now to make the remote branch the same as your local branch you can do git push --force-with-lease.

I assume your local branch now has the same commit history as the remote. 3 commits and a merge. To get it back to where you were before squashing/pushing/pulling/merging do you can do git log to find the hash string of the Simplified ui_init commit. Then you can do git reset --hard your-commit-hash-string-here. Then you should be back where you started and can do the process described above.

[kristjanbjarni]

@klutvott123 I fixed the commits.

Yes I can look into changing the code to use MCU id instead.

[kristjanbjarni]

OK changed the name to use instead the MSP_UID as 24 hex characters to make it short and consistant. Tested a couple of quads, seemed to work just fine.

[kristjanbjarni]

Updated to use mcuId and fixed the order of the id to be the same order as mcu_id in CLI. I removed the check for apiVersion>1.042 since although you can't download the vtx tables under that version you should still be able to manually copy them to the transmitter and use them with the MCU id, right?

[klutvott123]

@kristjanbjarni I think we should keep the check. Vtx tables were introduced in betaflight 4.1(api version 1.042) so I don't see why we would want to load vtx tables for firmware versions older than that. For that we have vtx_defaults.lua.

[kristjanbjarni]

@klutvott123 Added the check back in

[klutvott123]

Not sure if storing the model name in mcuId is the best thing to do. I would combine the conditionals like
elseif apiVersion >= 1.042 and not mcuId then
and leave mcuId = nilfor older versions.
This would require loading vtx tables or defaults in vtx.lua based on apiVersion like this:

local vtx_tables
if apiVersion >= 1.042 then
    vtx_tables = assert(loadScript("/BF/VTX/"..mcuId..".lua"))()
else
    vtx_tables = assert(loadScript("/BF/VTX/vtx_defaults.lua"))()
end

Can we change the message to just "Waiting for device ID"? That would make it fit nicely on the 128x64 screens too.

[klutvott123]

@kristjanbjarni Great! Just added some comments. We're almost there 😁

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

No Coverage information
No Duplication information

[klutvott123]

👍

[mikeller]

Nice work, thanks @kristjanbjarni and @klutvott123, this will put this problem to an end once and for all.

[klutvott123]

Edited PR title to make it clear that we're using mcu ID and not craft name. Just to avoid any confusion

[limonspb]

Sorry, late to the party and got nothing useful to say, but thanks a lot!!! This is brilliant.