Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mutual exclusive gyro alignment #644

Conversation

haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Dec 19, 2024

Copy link
Contributor

@ledvinap ledvinap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marked some problems, but not all.
My PR removes trivial cases, I can improve the script to handle FLIP too. Then remaining CUSTOM align needs individual review/fix

@@ -60,7 +60,6 @@
#define GYRO_1_CS_PIN PA4
#define GYRO_1_SPI_INSTANCE SPI1
#define GYRO_1_ALIGN CW0_DEG
#define GYRO_1_ALIGN_YAW 1800
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is wrong - only remove cases where GYRO_1_ALIGN and GYRO_1_ALIGN_YAW do match. These mismatches must be handled individually (or left to trigger error)

@@ -120,5 +120,4 @@

#define GYRO_1_SPI_INSTANCE SPI1
#define GYRO_1_ALIGN CW0_DEG_FLIP
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

total mismatch

@@ -106,4 +106,3 @@
#define FLASH_SPI_INSTANCE SPI3
#define GYRO_1_SPI_INSTANCE SPI1
#define GYRO_1_ALIGN CW0_DEG_FLIP
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!

@@ -123,8 +123,6 @@ Details: https://github.com/crteensy/yolo-chonker/tree/as-built-20230303

#define DEFAULT_GYRO_TO_USE GYRO_CONFIG_USE_GYRO_1
#define GYRO_1_ALIGN CW0_DEG_FLIP
#define GYRO_1_ALIGN_PITCH 1800
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(this one is correct - 0deg + flip)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would think to remove #define GYRO_1_ALIGN CW0_DEG_FLIP instead.

@@ -245,6 +245,3 @@
#define GYRO_1_ALIGN CW180_DEG
#define GYRO_2_SPI_INSTANCE SPI2
#define GYRO_2_ALIGN ALIGN_CUSTOM
#define GYRO_2_ALIGN_ROLL 0
#define GYRO_2_ALIGN_PITCH 0
#define GYRO_2_ALIGN_YAW 2250
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THis one must stay - it is intentional CUSTOM alignment

@@ -118,7 +118,6 @@
#define FLASH_SPI_INSTANCE SPI3
#define GYRO_1_SPI_INSTANCE SPI1
#define GYRO_1_ALIGN CW180_DEG
#define GYRO_1_ALIGN_PITCH 1800
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I'm confused why this is valid.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

180 pitch is cw0_flip

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for ignorance - as you stated settings are mutual exclusive - in my understanding when using GYRO_1_ALIGN CW180_DEG custom definition should be unavailable

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm talking about config.h - it should use either GYRO_1_ALIGN CW180_DEG or #define GYRO_1_ALIGN_YAW 1800. In this case, ALIGN is CW180_DEG but custom alignment is GYRO_1_ALIGN_PITCH 1800. This does not match, there is some problem with config.

@@ -146,7 +146,6 @@
#define PINIO1_BOX 40
#define GYRO_1_SPI_INSTANCE SPI1
#define GYRO_1_ALIGN CW180_DEG
#define GYRO_1_ALIGN_PITCH 1800
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!

@haslinghuis
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants