MSP Multiple GYRO/ACC alignment support #1338

fujin · 2019-04-02T05:43:18Z

Make use of 1.41.0 MSP API changes introduced
Update MSP gyro alignment support and preserve backwards compatibility betaflight#7506
Not really happy with the elements/css alignment/UX, but opening this up for feedback.

4.0.0-RC5 dual gyro board (MKF7SE) `USE_MULTI_GYRO`:

4.0.0-RC5 single gyro board (AKF4):

It looks like now that USE_MULTI_GYRO is the default, the use_multi_gyro MSP API 1.41.0 flag is sent regardless of the second IMU being present or not. We could perhaps improve this.

3.5.7 single gyro board (AKF4):

McGiverGim

Good job! Only minor changes :)

src/css/tabs/configuration.css

src/js/tabs/configuration.js

* Make use of 1.41.0 MSP API changes introduced betaflight/betaflight#7506 * Not really happy with the elements/css, but opening this up for feedback. I can't quite get the select inputs or the text/icons to go where I want :p

McGiverGim

Only a suggestions, but to me is right at it is.

I will try to test it later, to be sure that it works. We will release the Configurator soon so we will have very few feedback from users.

McGiverGim · 2019-04-02T07:22:54Z

src/js/tabs/configuration.js

+        var legacy_accel_alignment_e = $('.tab-configuration .legacy_accel_alignment');
+
+        // Hide the new multi gyro element by default
+        gyro_align_content_e.hide();


Maybe is better to include it in the else of the next if, to let it clear when is hide or shown. But is only an opinion. To me is right in both ways.

McGiverGim · 2019-04-02T08:07:39Z

Under some resolutions, maybe caused by the translations, it has some problems...

Reducing it more, it goes worst:

To try to fix it, I suggest:

Reduce the size of the three input for the degrees in the FC. It needs only 3 digits.
Maybe is better to split the gyro into two lines: one for the selection of the gyro, and other for the alignment of each gyro. Another solution is to fix the size of the select to something reasonable and put an overflow:ellipsis for the texts to not break into two lines.

What do you think?

fujin · 2019-04-03T00:00:20Z

@McGiverGim I'll try both of your suggestions, added Don't Merge label for now.

mikeller · 2019-04-03T00:14:31Z

Tested this with betaflight/betaflight#7914, and the behaviour with / without multiple gyros is working as expected. We could probably extend this to make it return (no gyro /) single gyro / multi gyro / dual gyro (multi & both the same type).

mikeller · 2019-04-03T02:25:04Z

Sorry, but I figured out we could as well go all in, and show the user if his setup is capable of 'BOTH' as well, considering we already do all of the detecting required for it. Now the first byte of the gyro data on MSP returns a value from this enum: https://github.com/betaflight/betaflight/pull/7914/files#diff-20ad1ec21e7f8cb08c6b7951c95049eaR71

McGiverGim · 2019-04-03T07:16:33Z

You are proposing to show BOTH in the select only if DETECTED_DUAL_GYROS is returned in this bit, right?

EDIT: and I suppose the same for FIRST (DETECTED_GYRO_1 | DETECTED_BOTH_GYROS | DETECTED_DUAL_GYROS ) and SECOND (DETECTED_GYRO_2 | DETECTED_BOTH_GYROS | DETECTED_DUAL_GYROS ).

mikeller · 2019-04-03T07:23:28Z

@McGiverGim: The returned byte will contain:

a bit flag for every detected gyro (max 2 for now);
a separate bit flag if all (2) of the detected gyros are of the same type and hence can be used together.

We've already had a bunch of user requests for 'why can't my 2 gyros (which are of a different model) be used for both'. The hope is that if we make the GUI aware of this, and potentially add a help text explaining to users that why it works this way, we can cut down on these requests.

fujin · 2019-04-03T19:58:57Z

The both/dual stuff is epic, that will definitely help with the UX. I'll incorporate into this change. It will definitely help the case where dual gyro board cannot use 'BOTH' e.g. sensor fusion and configuration is silently reset.

…ignment Fix styling board and gyro align

fujin · 2019-04-05T08:34:28Z

Good to go 👍 thanks @McGiverGim !!

McGiverGim · 2019-04-05T09:55:46Z

I have almost ready the version that shows/hides the BOTH and GYRO depending on the flags. Is better to make a PR after this is merged o make the PR to your branch @fujin?

fujin · 2019-04-05T10:05:18Z

@McGiverGim either is fine with me. I'm heading to sleep soon so maybe makes sense to do a separate one.

McGiverGim · 2019-04-05T10:12:32Z

Yes, after doing some tests, I think there are some things that maybe need discussion. So maybe better to merge this first, and then, I will push the other to see what is the final desired behaviour.

mikeller

Approving this under the provisio that #1346 fixes the misalignment of use_multi_gyro.

mikeller · 2019-04-07T04:38:47Z

src/js/tabs/configuration.js

+        gyro_align_content_e.hide();
+
+        // If we are sent USE_MULTI_GYRO flag from the target, show the new element, while hiding the old ones.
+        if (SENSOR_ALIGNMENT.use_multi_gyro == 1) {


This is not in accordance with what Betaflight 4.0.0-RC6 is sending for use_multi_gyro.

fujin requested review from McGiverGim and etracer65 April 2, 2019 05:43

fujin force-pushed the improve-board-and-gyro-alignment branch 3 times, most recently from 24a9aaf to 8d99a1c Compare April 2, 2019 06:44

McGiverGim requested changes Apr 2, 2019

View reviewed changes

src/css/tabs/configuration.css Outdated Show resolved Hide resolved

src/js/tabs/configuration.js Show resolved Hide resolved

src/js/tabs/configuration.js Show resolved Hide resolved

fujin force-pushed the improve-board-and-gyro-alignment branch from 8d99a1c to e5da4a3 Compare April 2, 2019 06:59

MSP Multiple GYRO/ACC alignment support

e322ef0

* Make use of 1.41.0 MSP API changes introduced betaflight/betaflight#7506 * Not really happy with the elements/css, but opening this up for feedback. I can't quite get the select inputs or the text/icons to go where I want :p

fujin force-pushed the improve-board-and-gyro-alignment branch from e5da4a3 to e322ef0 Compare April 2, 2019 07:03

fujin requested a review from McGiverGim April 2, 2019 07:16

McGiverGim previously approved these changes Apr 2, 2019

View reviewed changes

fujin added the Don't merge label Apr 2, 2019

McGiverGim and others added 2 commits April 5, 2019 10:14

Fix styling board and gyro align

53b18b6

Merge pull request #1 from McGiverGim/fujin-improve-board-and-gyro-al…

f002232

…ignment Fix styling board and gyro align

fujin dismissed McGiverGim’s stale review via f002232 April 5, 2019 08:33

fujin removed the Don't merge label Apr 5, 2019

McGiverGim approved these changes Apr 5, 2019

View reviewed changes

McGiverGim mentioned this pull request Apr 5, 2019

Show/Hide gyro elements #1346

Merged

McGiverGim added this to the 10.5.0 milestone Apr 5, 2019

mikeller approved these changes Apr 7, 2019

View reviewed changes

mikeller merged commit 06c2044 into betaflight:master Apr 7, 2019

mikeller added the RN: UI label Apr 9, 2019

Uh oh!

MSP Multiple GYRO/ACC alignment support #1338

MSP Multiple GYRO/ACC alignment support #1338

Uh oh!

Conversation

fujin commented Apr 2, 2019

4.0.0-RC5 dual gyro board (MKF7SE) USE_MULTI_GYRO:

4.0.0-RC5 single gyro board (AKF4):

3.5.7 single gyro board (AKF4):

Uh oh!

McGiverGim left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

McGiverGim left a comment

Choose a reason for hiding this comment

Uh oh!

McGiverGim Apr 2, 2019

Choose a reason for hiding this comment

Uh oh!

McGiverGim commented Apr 2, 2019

Uh oh!

fujin commented Apr 3, 2019

Uh oh!

mikeller commented Apr 3, 2019

Uh oh!

mikeller commented Apr 3, 2019

Uh oh!

McGiverGim commented Apr 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikeller commented Apr 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fujin commented Apr 3, 2019

Uh oh!

fujin commented Apr 5, 2019

Uh oh!

McGiverGim commented Apr 5, 2019

Uh oh!

fujin commented Apr 5, 2019

Uh oh!

McGiverGim commented Apr 5, 2019

Uh oh!

mikeller left a comment

Choose a reason for hiding this comment

Uh oh!

mikeller Apr 7, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

4.0.0-RC5 dual gyro board (MKF7SE) `USE_MULTI_GYRO`:

McGiverGim commented Apr 3, 2019 •

edited

Loading

mikeller commented Apr 3, 2019 •

edited

Loading