Mcu ID for vtx tables by kristjanbjarni · Pull Request #361 · betaflight/betaflight-tx-lua-scripts · GitHub

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.

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.

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

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?

@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.

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

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.

@klutvott123 I fixed the commits.

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

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.

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?

@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.

@klutvott123 Added the check back in

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.

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

Reads MCU id from MSP and uses that unique id to store the vtx tables. This should solve the problem of using an incorrect vtx table when there are different crafts with different vtx hardware under the same model name.

Kudos, SonarCloud Quality Gate passed!

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

No Coverage information
No Duplication information

👍

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

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

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

klutvott123 reviewed Sep 8, 2020

View reviewed changes

Comment thread src/SCRIPTS/BF/ui_init.lua Outdated

Comment thread src/SCRIPTS/BF/ui_init.lua Outdated

Comment thread src/SCRIPTS/BF/ui_init.lua Outdated

klutvott123 added this to the 1.6 milestone Sep 12, 2020

klutvott123 added the RN: IMPROVEMENT label Sep 12, 2020

kristjanbjarni force-pushed the craft_name branch from ee32561 to 703c7bf Compare September 14, 2020 22:48

kristjanbjarni force-pushed the craft_name branch from 703c7bf to 4b87e63 Compare September 15, 2020 23:36

klutvott123 reviewed Sep 16, 2020

View reviewed changes

Comment thread src/SCRIPTS/BF/craft_name.lua Outdated

Comment thread src/SCRIPTS/TOOLS/bf.lua Outdated

kristjanbjarni force-pushed the craft_name branch from 4b87e63 to b207b1f Compare September 17, 2020 20:50

kristjanbjarni force-pushed the craft_name branch from b207b1f to 06b4e5d Compare September 19, 2020 18:59

klutvott123 reviewed Sep 19, 2020

View reviewed changes

MCU id for vtx tables

568595d

Reads MCU id from MSP and uses that unique id to store the vtx tables. This should solve the problem of using an incorrect vtx table when there are different crafts with different vtx hardware under the same model name.

kristjanbjarni force-pushed the craft_name branch from 06b4e5d to 568595d Compare September 19, 2020 22:03

klutvott123 approved these changes Sep 19, 2020

View reviewed changes

mikeller approved these changes Sep 20, 2020

View reviewed changes

mikeller merged commit 335a9d4 into betaflight:master Sep 20, 2020

kristjanbjarni deleted the craft_name branch December 29, 2020 13:45

klutvott123 changed the title Craft name for vtx tables Mcu ID for vtx tables Apr 9, 2021

+                  elseif not mcuId then
+                      if apiVersion >= 1.042 then
+                          lcd.drawText(6, radio.yMinLimit, "Waiting for unique device ID")
+                          getMCUId = getMCUId or assert(loadScript("mcu_id.lua"))()
+                          if getMCUId() then
+                              getMCUId = nil
+                              local vtxTables = loadScript("/BF/VTX/"..mcuId..".lua")
+                              if vtxTables and vtxTables() then
+                                  vtxTablesReceived = true
+                                  vtxTables = nil
+                              end
+                              collectgarbage()
+                          end
+                      else
+                          mcuId = model.getInfo().name
+                      end

Mcu ID for vtx tables by kristjanbjarni · Pull Request #361 · betaflight/betaflight-tx-lua-scripts · GitHub

Uh oh!

Mcu ID for vtx tables - #361

Mcu ID for vtx tables#361mikeller merged 1 commit intobetaflight:masterbetaflight/betaflight-tx-lua-scripts:masterfrom kristjanbjarni:craft_namekristjanbjarni/betaflight-tx-lua-scripts:craft_nameCopy head branch name to clipboard

Conversation

kristjanbjarni commented Sep 8, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kristjanbjarni commented Sep 9, 2020

Uh oh!

klutvott123 commented Sep 12, 2020

Uh oh!

klutvott123 commented Sep 12, 2020

Uh oh!

kristjanbjarni commented Sep 12, 2020

Uh oh!

mikeller commented Sep 13, 2020

Uh oh!

klutvott123 commented Sep 13, 2020

Uh oh!

kristjanbjarni commented Sep 14, 2020

Uh oh!

kristjanbjarni commented Sep 16, 2020

Uh oh!

Uh oh!

Uh oh!

kristjanbjarni commented Sep 17, 2020

Uh oh!

klutvott123 commented Sep 18, 2020

Uh oh!

kristjanbjarni commented Sep 19, 2020

Uh oh!

klutvott123 Sep 19, 2020

Choose a reason for hiding this comment

Uh oh!

klutvott123 commented Sep 19, 2020

Uh oh!

sonarqubecloud Bot commented Sep 19, 2020

Uh oh!

klutvott123 left a comment

Choose a reason for hiding this comment

Uh oh!

mikeller left a comment

Choose a reason for hiding this comment

Uh oh!

klutvott123 commented Apr 9, 2021

Uh oh!

limonspb commented Jul 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Mcu ID for vtx tables#361
mikeller merged 1 commit into
betaflight:masterfrom
kristjanbjarni:craft_name

limonspb commented Jul 16, 2021 •
edited
Loading