Trim acc #457

druckgott · 2022-11-12T16:41:09Z

Added A new Page fro Trim Accelerometer

Add new Page: title = "Trim Accelerometer", script = "acc_calibrate.lua"

Add Page to Trim Accelerometer: MSP_ACC_TRIM MSP_SET_ACC_TRIM { "acc_trim_pitch", VAR_INT16 | MASTER_VALUE, .config.minmax = { -300, 300 }, PG_ACCELEROMETER_CONFIG, offsetof(accelerometerConfig_t, accelerometerTrims.values.pitch) }, { "acc_trim_roll", VAR_INT16 | MASTER_VALUE, .config.minmax = { -300, 300 }, PG_ACCELEROMETER_CONFIG, offsetof(accelerometerConfig_t, accelerometerTrims.values.roll) },

haslinghuis · 2022-11-12T19:58:35Z

src/SCRIPTS/BF/PAGES/acc_calibrate.lua

+local labels = {}
+local fields = {}
+
+if apiVersion >= 1.041 then


Suggested change

if apiVersion >= 1.041 then

if apiVersion >= 1.16 then

klutvott123 · 2022-11-12T19:53:31Z

src/SCRIPTS/BF/PAGES/acc_calibrate.lua

@@ -0,0 +1,30 @@
+


Don't want this

klutvott123 · 2022-11-12T19:54:15Z

src/SCRIPTS/BF/PAGES/acc_calibrate.lua

+local labels = {}
+local fields = {}
+
+if apiVersion >= 1.041 then


These values have been available through msp in betaflight since msp api version 1.16(we had msp over telemetry since then) so this conditional is not needed.

We only need conditionals like this in the page file itself if parameters are removed/added to the message along the way.
In this case the only conditional needed is the one in "pages.lua" that makes the whole page conditional to msp api version 1.16

What would happen here is that for api versions lower than 1.41 the page would still be accessible, but would be empty.

klutvott123 · 2022-11-12T20:01:36Z

src/SCRIPTS/BF/PAGES/acc_calibrate.lua

+if apiVersion >= 1.041 then
+	labels[#labels + 1] = { t = "Trim Accelerometer" ,     x = x,          y = inc.y(lineSpacing) }
+	fields[#fields + 1] = { t = "acc trim pitch",    x = x, y = inc.y(lineSpacing), sp = x + sp, min = -300, max = 300, vals = { 1 } }
+	fields[#fields + 1] = { t = "acc trim roll",     x = x, y = inc.y(lineSpacing), sp = x + sp, min = -300, max = 300, vals = { 2 } }


Too much indent.
The field text could also be just "Pitch" and "Roll" since the "Trim Accelerometer" header explains it all. These should also be indented. "x = x + indent" for both of these. Look at the other pages to see how we usually do it.

This won't work as it should. both pitch and roll trim are 16 bit values so for pitch it should be "vals = { 1, 2 }" and for roll "vals = { 3, 4 }". Have a look at the MSP_ACC_TRIM" message in the BF source code. 🙂

klutvott123 · 2022-11-12T20:02:37Z

src/SCRIPTS/BF/pages.lua

@@ -56,4 +56,8 @@ if apiVersion >= 1.41 then
    PageFiles[#PageFiles + 1] = { title = "GPS PIDs", script = "gpspids.lua" }
 end

+if apiVersion >= 1.16 then
+    PageFiles[#PageFiles + 1] = { title = "Trim Accelerometer", script = "acc_calibrate.lua" }


maybe "acc_trim.lua" would be a more appropriate filename since we already have similarly named files to calibrate the accelerometer.

klutvott123 · 2022-11-12T20:05:09Z

src/SCRIPTS/BF/PAGES/acc_calibrate.lua

+    title       = "ACC",
+    reboot      = true,
+    eepromWrite = true,
+    minBytes    = 1,


For this message we need at least 4 bytes to be able to do anything.

klutvott123 · 2022-11-12T20:05:31Z

src/SCRIPTS/BF/PAGES/acc_calibrate.lua

+    read        = 240, -- MSP_ACC_TRIM
+    write       = 239, -- MSP_SET_ACC_TRIM         
+    title       = "ACC",
+    reboot      = true,


Is a reboot needed for the trim to take effect?

I don't know if it's needed. How can I test this?

Thx for all the other informations it's my fist commit,didnt know all the thinks only test it and it works 😉.

What happens if I do Wirte only 1 instead of 1, 2 due to 16 bit?

"reboot = false". Then try making some changes, save the page and see if it took effect 🙂

What happens is that the pitch field will modify the first byte of pitch trim and the roll field will modify the second byte of pitch trim. So the roll field will make unintended changes to pitch trim, and you won't be able to change roll trim at all

1.remove first line 2. remove: if apiVersion >= 1.041 then ... 3. fix indent 4. add: x = x + indent 5. change name acc_calibrate.lua --> acc_trim.lua 6. reboot = false, 7. minBytes = 4, 8. fix vals

remove due to name change

druckgott · 2022-11-12T21:27:10Z

Fixed all points and tested it. Worked for me.

klutvott123 · 2022-11-12T21:43:45Z

Negative values doesn't display properly. We haven't had to deal with negative values in the bf lua scripts until now so everything is treated as unsigned. We'll have to look into that

klutvott123 · 2022-11-12T23:10:36Z

src/SCRIPTS/BF/PAGES/acc_trim.lua

+labels[#labels + 1] = { t = "Trim Accelerometer",	x = x,          y = inc.y(lineSpacing) }
+fields[#fields + 1] = { t = "Pitch",    			x = x + indent, y = inc.y(lineSpacing), sp = x + sp, min = -300, max = 300, vals = { 1, 2 } }
+fields[#fields + 1] = { t = "Roll",     			x = x + indent, y = inc.y(lineSpacing), sp = x + sp, min = -300, max = 300, vals = { 3, 4 } }


Looks like you're using tabs here. use spaces to align things.

Change Tab to Space

druckgott · 2022-11-13T08:16:24Z

Ok yes I also so this this -1 ore <=-1, do you have an Idea where we have to change the code to get this working. Is there any way ob debuggin?

druckgott · 2022-11-13T09:53:41Z

Ok now the problem with negativ values is fixed. I do not exactly know there the limit is by switching to negativ calculation so I selected 65000 because this will never reached from -300 to 300

klutvott123 · 2022-11-13T15:28:49Z

@druckgott I think it's better if we handle this in a more generic way. See #458
With this your PR should work without the postLoad function

druckgott · 2022-11-13T15:39:03Z

ok I removed it again and if your fix is done then it should be fine.

klutvott123 · 2022-11-13T15:47:59Z

Looks good to me. You just need to squash to one commit now 👍

druckgott · 2022-11-13T15:54:39Z

How can I squash to one commit now?

klutvott123 · 2022-11-13T16:32:04Z

git rebase -i HEAD~9
you'll get a list of the last 9 commits and a bunch of actions. every commit is prepended with "pick" as default. Replace "pick" with "fixup" or "f" for all commits except the top one that you want to keep. This will leave you with one commit with all your changes.
Then git push --force-with-lease to push your changes

druckgott · 2022-11-13T18:21:31Z

Mhh I cant get it working. I did all the things bevor online direct on github.com and not with git clone ....
can you fix it, next time I will directly do it with git on windows.

klutvott123 · 2022-11-13T20:59:24Z

Just make a new PR

druckgott · 2022-11-14T07:03:04Z

Did a new one:
#459

druckgott added 2 commits November 12, 2022 17:37

Add Page: Trim Accelerometer

21ead4f

Add new Page: title = "Trim Accelerometer", script = "acc_calibrate.lua"

haslinghuis reviewed Nov 12, 2022

View reviewed changes

klutvott123 requested changes Nov 12, 2022

View reviewed changes

druckgott added 3 commits November 12, 2022 22:22

Update name: acc_trim.lua

1535b18

Fix Points in File

a8d0765

1.remove first line 2. remove: if apiVersion >= 1.041 then ... 3. fix indent 4. add: x = x + indent 5. change name acc_calibrate.lua --> acc_trim.lua 6. reboot = false, 7. minBytes = 4, 8. fix vals

Delete acc_calibrate.lua

3383fd7

remove due to name change

klutvott123 requested changes Nov 12, 2022

View reviewed changes

Update acc_trim.lua

b19b6a6

Change Tab to Space

Fix problem with negativ values

1a9b57f

size down calculation

d131e74

druckgott requested review from klutvott123 and haslinghuis and removed request for klutvott123 November 13, 2022 11:13

remove postload due to generic way

d25763d

druckgott mentioned this pull request Nov 14, 2022

Trim acc2 #459

Merged

druckgott closed this Nov 14, 2022

druckgott mentioned this pull request Dec 5, 2022

Position OSD Elements via Script #463

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Trim acc #457

Trim acc #457

druckgott commented Nov 12, 2022

haslinghuis Nov 12, 2022

klutvott123 Nov 12, 2022

klutvott123 Nov 12, 2022

klutvott123 Nov 12, 2022

klutvott123 Nov 12, 2022

klutvott123 Nov 12, 2022

klutvott123 Nov 12, 2022

druckgott Nov 12, 2022

klutvott123 Nov 12, 2022

druckgott commented Nov 12, 2022

klutvott123 commented Nov 12, 2022

klutvott123 Nov 12, 2022

druckgott commented Nov 13, 2022

druckgott commented Nov 13, 2022 •

edited

Loading

klutvott123 commented Nov 13, 2022

druckgott commented Nov 13, 2022

klutvott123 commented Nov 13, 2022

druckgott commented Nov 13, 2022

klutvott123 commented Nov 13, 2022

druckgott commented Nov 13, 2022

klutvott123 commented Nov 13, 2022

druckgott commented Nov 14, 2022

Trim acc #457

Trim acc #457

Conversation

druckgott commented Nov 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

druckgott commented Nov 12, 2022

klutvott123 commented Nov 12, 2022

Choose a reason for hiding this comment

druckgott commented Nov 13, 2022

druckgott commented Nov 13, 2022 • edited Loading

klutvott123 commented Nov 13, 2022

druckgott commented Nov 13, 2022

klutvott123 commented Nov 13, 2022

druckgott commented Nov 13, 2022

klutvott123 commented Nov 13, 2022

druckgott commented Nov 13, 2022

klutvott123 commented Nov 13, 2022

druckgott commented Nov 14, 2022

druckgott commented Nov 13, 2022 •

edited

Loading