Fix missing ACCGYRO custom alignment defines (used in config) #14092

haslinghuis · 2024-12-18T18:04:38Z

we have migrated custom alignment from unified targets using

GYRO_1_ALIGN_ROLL 
GYRO_1_ALIGN_PITCH
GYRO_1_ALIGN_YAW

but they are not being used

Learned we either use CUSTOM setting with these settings or use the preset (GYRO_1_ALIGN)
Configuration repository is updated to reflect this in GYRO_x_ALIGN_YAW config#645
Updated documentation https://betaflight.com/docs/development/manufacturer/creating-configuration#gyro-alignment-settings

github-actions · 2024-12-18T18:04:51Z

Do you want to test this code? You can flash it directly from Betaflight Configurator:

Simply put #14092 (this pull request number) in the Select commit field of the Configurator firmware flasher tab (you need to Enable expert mode, Show release candidates and Development).

WARNING: It may be unstable. Use only for testing!

ledvinap · 2024-12-18T18:26:11Z

src/main/common/sensor_alignment.h

-#define CUSTOM_ALIGN_CW90_DEG_FLIP   SENSOR_ALIGNMENT(  0, 180,  90)
-#define CUSTOM_ALIGN_CW180_DEG_FLIP  SENSOR_ALIGNMENT(  0, 180, 180)
-#define CUSTOM_ALIGN_CW270_DEG_FLIP  SENSOR_ALIGNMENT(  0, 180, 270)
+#define CUSTOM_ALIGN_CW0_DEG         SENSOR_ALIGNMENT( 0,   0,   0 )


I'd keep only 8 basic alignments and use macro for rest:

#define CUSTOM_ALIGN_CW(deg) SENSOR_ALIGNMENT(0, 0, (deg)); #define CUSTOM_ALIGN_CW_FLIP(deg) SENSOR_ALIGNMENT(0, 180, (deg)); #define CUSTOM_ALIGN_CW0_DEG CUSTOM_ALIGN_CW(0) #define CUSTOM_ALIGN_CW90_DEG_FLIP CUSTOM_ALIGN_CW_FLIP(90)

This would be

// #define CUSTOM_ALIGN_CW0_DEG SENSOR_ALIGNMENT( 0, 0, 0 ) #define CUSTOM_ALIGN_CW90_DEG SENSOR_ALIGNMENT( 0, 0, 90 ) #define CUSTOM_ALIGN_CW180_DEG SENSOR_ALIGNMENT( 0, 0, 180 ) #define CUSTOM_ALIGN_CW270_DEG SENSOR_ALIGNMENT( 0, 0, 270 ) #define CUSTOM_ALIGN_CW0_DEG_FLIP SENSOR_ALIGNMENT( 0, 180, 0 ) #define CUSTOM_ALIGN_CW45_DEG_FLIP SENSOR_ALIGNMENT( 0, 180, 45 ) // #define CUSTOM_ALIGN_CW90_DEG_FLIP SENSOR_ALIGNMENT( 0, 180, 90 ) #define CUSTOM_ALIGN_CW180_DEG_FLIP SENSOR_ALIGNMENT( 0, 180, 180 ) #define CUSTOM_ALIGN_CW270_DEG_FLIP SENSOR_ALIGNMENT( 0, 180, 270 ) #define CUSTOM_ALIGN_CW(deg) SENSOR_ALIGNMENT( 0, 0, (deg) ) #define CUSTOM_ALIGN_CW_FLIP(deg) SENSOR_ALIGNMENT( 0, 180, (deg) ) #define CUSTOM_ALIGN_CW0_DEG CUSTOM_ALIGN_CW( 0 ) #define CUSTOM_ALIGN_CW90_DEG_FLIP CUSTOM_ALIGN_CW_FLIP( 90 )

It is IMO better to cascade it. That way, it is easier to change CUSTOM_ALIGN internals

#define CUSTOM_ALIGN_CW90_DEG CUSTOM_ALIGN_CW(90) ....

ledvinap · 2024-12-18T18:29:22Z

src/main/common/sensor_alignment.h

@@ -61,14 +61,22 @@ typedef union sensorAlignment_u {
    .yaw = DEGREES_TO_DECIDEGREES(YAW), \


Above:

#define SENSOR_ALIGNMENT(ROLL, PITCH, YAW) ((const sensorAlignment_t){\

At least in some situations it generated considerably better code

Add new one

haslinghuis · 2024-12-18T18:55:08Z

~~Consensus~~ Idea is too use SENSOR_ALIGN( 0, 180, 270) // 270 flip directly in configuration as this would be easier to use for manufacturers

ledvinap · 2024-12-18T18:58:10Z

(please don't force-push this PR now - I'm working on PR against your branch)

(edited - original version had wrong interpretation)

src/main/pg/gyrodev.c

haslinghuis · 2024-12-18T19:29:36Z

Another idea would be to use current defines for filling up SENSOR_ALIGNMENT define

ledvinap · 2024-12-18T19:42:54Z

@haslinghuis : Look at haslinghuis#7

If GYRO_x_ALIGN is defined, it is used to fill both alignment and customAlignment.
When GYRO_1_CUSTOM_ALIGN is set, GYRO_1_ALIGN must be either undefined or ALIGN_CUSTOM and GYRO_1_CUSTOM_ALIGN is used for customAlignment
When both are defined, code enforces that GYRO_1_ALIGN == ALIGN_CUSTOM (needs testing)

This should IMO handle intended use and allow easy transition from STD align in code

ledvinap · 2024-12-18T19:43:18Z

@haslinghuis : The code is not tested .

haslinghuis · 2024-12-18T19:49:10Z

Agree. Custom defines in first message were not being used. Configuration repository needs update.
Two situations:

migrated configurations from unified targets
new targets defined custom alignments but were not working due to missing definition

haslinghuis · 2024-12-18T21:06:25Z

Now looking at

https://github.com/betaflight/config/blob/e6d28381c61fb42fdca9029d4e1ccbc794e64632/configs/SPRACINGH7EXTREME/config.h#L247-L250

haslinghuis · 2024-12-18T23:47:56Z

@ledvinap think best uniform way this should be used as (to simplify configuration):

#define GYRO_1_CUSTOM_ALIGN SENSOR_ALIGNMENT( ROLL, PITCH, YAW )
#define GYRO_2_CUSTOM_ALIGN SENSOR_ALIGNMENT( ROLL, PITCH, YAW )

haslinghuis · 2024-12-19T00:11:53Z

@ledvinap to retain configuration defines in config repository how about

#ifndef GYRO_1_ALIGN_ROLL
#define GYRO_1_ALIGN_ROLL 0
#endif

#ifndef GYRO_1_ALIGN_PITCH
#define GYRO_1_ALIGN_PITCH 0
#endif

#ifndef GYRO_1_ALIGN_YAW
#define GYRO_1_ALIGN_YAW 0
#endif

#ifndef GYRO_2_ALIGN_ROLL
#define GYRO_2_ALIGN_ROLL 0
#endif

#ifndef GYRO_2_ALIGN_PITCH
#define GYRO_2_ALIGN_PITCH 0
#endif

#ifndef GYRO_2_ALIGN_YAW
#define GYRO_2_ALIGN_YAW 0
#endif

#define GYRO_1_CUSTOM_ALIGN SENSOR_ALIGNMENT( GYRO_1_ALIGN_ROLL, GYRO_1_ALIGN_PITCH, GYRO_1_ALIGN_YAW )

#define GYRO_2_CUSTOM_ALIGN SENSOR_ALIGNMENT( GYRO_2_ALIGN_ROLL, GYRO_2_ALIGN_PITCH, GYRO_2_ALIGN_YAW )

ledvinap · 2024-12-19T09:06:13Z

https://github.com/betaflight/config/blob/e6d28381c61fb42fdca9029d4e1ccbc794e64632/configs/SPRACINGH7EXTREME/config.h#L247-L250

#define GYRO_2_CUSTOM_ALIGN CUSTOM_ALIGN_CW(225)

ledvinap · 2024-12-19T09:09:53Z

@ledvinap to retain configuration defines in config repository how about

there are 2 targets using custom align, it is easier to fix them
all three GYRO_1_ALIGN_x are defined together, so

#def GYRO_1_ALIGN_ROLL
# ifndef GYRO_1_CUSTOM_ALIGN 
#  define GYRO_1_CUSTOM_ALIGN SENSOR_ALIGNMENT( GYRO_1_ALIGN_ROLL, GYRO_1_ALIGN_PITCH, GYRO_1_ALIGN_YAW )
# else
#  error "GYRO_1_ALIGN_x and GYRO_1_CUSTOM_ALIGN  are mutually exclusive"
# endif
#endif

ledvinap · 2024-12-19T09:12:57Z

@haslinghuis:

SENSOR_ALIGNMENT_FROM_STD is not necessary now, but maybe keep it - custom alignment parameters will be always valid ant it can be later used to unify alignment, scaling and offset compensation.

src/main/pg/gyrodev.c

ledvinap · 2024-12-19T15:04:21Z

src/main/common/sensor_alignment.h

-#define CUSTOM_ALIGN_CW90_DEG_FLIP   SENSOR_ALIGNMENT(  0, 180,  90)
-#define CUSTOM_ALIGN_CW180_DEG_FLIP  SENSOR_ALIGNMENT(  0, 180, 180)
-#define CUSTOM_ALIGN_CW270_DEG_FLIP  SENSOR_ALIGNMENT(  0, 180, 270)
+#define CUSTOM_ALIGN_CW0_DEG         SENSOR_ALIGNMENT( 0,   0,   0 )


It is IMO better to cascade it. That way, it is easier to change CUSTOM_ALIGN internals

#define CUSTOM_ALIGN_CW90_DEG CUSTOM_ALIGN_CW(90) ....

src/main/pg/gyrodev.c

nerdCopter · 2024-12-19T18:53:57Z

grep -r -Z -l 'GYRO_1_ALIGN ' . | xargs -0 grep 'GYRO_1_ALIGN_'

there are some GYRO_2_ as well
grep -r -Z -l 'GYRO_[12]_ALIGN ' . | xargs -0 grep 'GYRO_[12]_ALIGN_'

src/main/pg/gyrodev.c

src/main/common/sensor_alignment.h

src/main/pg/gyrodev.c

nerdCopter

approving for workflow, not tested, only visually reviewed.

src/main/pg/gyrodev.c

haslinghuis · 2025-01-10T19:12:50Z

@blckmn please approve / review

…ight#14092)

* Fix missing ACCGYRO custom alignment defines (used in config) (#14092) * Remove CLKIN pin not available in 4.5 * Update src/main/common/sensor_alignment_impl.h Co-authored-by: Petr Ledvina <ledvinap@gmail.com> --------- Co-authored-by: Petr Ledvina <ledvinap@gmail.com>

Fix missing align defines

2587e30

haslinghuis added the RN: BUGFIX label Dec 18, 2024

haslinghuis added this to the 4.6 milestone Dec 18, 2024

haslinghuis self-assigned this Dec 18, 2024

haslinghuis added 2 commits December 18, 2024 19:17

Add 45 degree options

e0bef11

Fix unused variable

5f48253

haslinghuis changed the title ~~Fix missing align defines~~ Fix missing ACCGYRO custom alignment defines Dec 18, 2024

ledvinap previously approved these changes Dec 18, 2024

View reviewed changes

Add new one

dfe74b1

haslinghuis commented Dec 18, 2024

View reviewed changes

src/main/pg/gyrodev.c Outdated Show resolved Hide resolved

Add code suggestion from ledvinap

7e6c5df

Restore CUSTOM_ALIGN defines in favor of SENSOR_ALIGNMENT macro

0105a8e

Make use of current configuration defines

8dd3fff

ledvinap reviewed Dec 19, 2024

View reviewed changes

src/main/pg/gyrodev.c Outdated Show resolved Hide resolved

Fix unit, remove comments, cascade custom align macros

b125c84

Do not redefine macros

1f412ee

haslinghuis mentioned this pull request Dec 19, 2024

Fix mutual exclusive gyro alignment betaflight/config#644

Closed

ledvinap reviewed Dec 20, 2024

View reviewed changes

src/main/pg/gyrodev.c Show resolved Hide resolved

Add GYRO_2_ALIGN_[RPY] defines

5914e0b

haslinghuis mentioned this pull request Dec 20, 2024

GYRO_x_ALIGN_YAW betaflight/config#645

Merged

haslinghuis requested review from ledvinap, blckmn, SteveCEvans, ctzsnooze, KarateBrot and klutvott123 December 20, 2024 21:26

ledvinap approved these changes Dec 21, 2024

View reviewed changes

src/main/common/sensor_alignment.h Outdated Show resolved Hide resolved

src/main/pg/gyrodev.c Show resolved Hide resolved

Cleanup commented code

7d4a973

nerdCopter approved these changes Dec 21, 2024

View reviewed changes

haslinghuis changed the title ~~Fix missing ACCGYRO custom alignment defines~~ Fix missing ACCGYRO custom alignment defines (used in config) Jan 2, 2025

ledvinap approved these changes Jan 9, 2025

View reviewed changes

src/main/pg/gyrodev.c Show resolved Hide resolved

blckmn approved these changes Jan 17, 2025

View reviewed changes

blckmn merged commit c188a03 into betaflight:master Jan 17, 2025
27 checks passed

haslinghuis added a commit to haslinghuis/betaflight that referenced this pull request Jan 17, 2025

Fix missing ACCGYRO custom alignment defines (used in config) (betafl…

a5db9da

…ight#14092)

haslinghuis mentioned this pull request Jan 17, 2025

[4.5.2] Fix missing ACCGYRO custom alignment defines (#14092) #14178

Merged

haslinghuis deleted the add-missing-align-defines branch April 23, 2025 14:33

		@@ -61,14 +61,22 @@ typedef union sensorAlignment_u {
		.yaw = DEGREES_TO_DECIDEGREES(YAW), \

Uh oh!

Fix missing ACCGYRO custom alignment defines (used in config) #14092

Fix missing ACCGYRO custom alignment defines (used in config) #14092

Uh oh!

Conversation

haslinghuis commented Dec 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 18, 2024

Uh oh!

ledvinap Dec 18, 2024

Choose a reason for hiding this comment

Uh oh!

haslinghuis Dec 18, 2024

Choose a reason for hiding this comment

Uh oh!

ledvinap Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

ledvinap Dec 18, 2024

Choose a reason for hiding this comment

Uh oh!

haslinghuis commented Dec 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ledvinap commented Dec 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

haslinghuis commented Dec 18, 2024

Uh oh!

ledvinap commented Dec 18, 2024

Uh oh!

ledvinap commented Dec 18, 2024

Uh oh!

haslinghuis commented Dec 18, 2024

Uh oh!

haslinghuis commented Dec 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

haslinghuis commented Dec 18, 2024

Uh oh!

haslinghuis commented Dec 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ledvinap commented Dec 19, 2024

Uh oh!

ledvinap commented Dec 19, 2024

Uh oh!

ledvinap commented Dec 19, 2024

Uh oh!

Uh oh!

Uh oh!

ledvinap Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nerdCopter commented Dec 19, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nerdCopter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

haslinghuis commented Jan 10, 2025

Uh oh!

Uh oh!

Uh oh!

haslinghuis commented Dec 18, 2024 •

edited

Loading

haslinghuis commented Dec 18, 2024 •

edited

Loading

ledvinap commented Dec 18, 2024 •

edited

Loading

haslinghuis commented Dec 18, 2024 •

edited

Loading

haslinghuis commented Dec 19, 2024 •

edited

Loading