Skip to content

Run CI for new boards with own defines #443

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

Merged
merged 16 commits into from
May 23, 2025
Merged

Run CI for new boards with own defines #443

merged 16 commits into from
May 23, 2025

Conversation

Eirenliel
Copy link
Member

@Eirenliel Eirenliel commented May 12, 2025

Makes CI run for new boards, but this time it also sets defines correctly, so the FW can be used in updates, and it also gets properly tested. It also builds glove fw for fun of it, and we can just set whatever defines we want to test in the matrix and it will do it.

I also "by accident" reorganized stuff a bit...

@Eirenliel Eirenliel marked this pull request as ready for review May 12, 2025 14:39
@Eirenliel Eirenliel requested a review from loucass003 as a code owner May 12, 2025 14:39
@Eirenliel Eirenliel requested review from gorbit99 and kounocom May 12, 2025 14:39
Comment on lines +29 to 46
#ifndef IMU
#define IMU IMU_AUTO
#endif
#ifndef SECOND_IMU
#define SECOND_IMU IMU_AUTO
#endif
#ifndef BOARD
#define BOARD BOARD_SLIMEVR_V1_2
#endif
#ifndef IMU_ROTATION
#define IMU_ROTATION DEG_270
#endif
#ifndef SECOND_IMU_ROTATION
#define SECOND_IMU_ROTATION DEG_270
#endif

#ifndef PRIMARY_IMU_OPTIONAL
#define PRIMARY_IMU_OPTIONAL false
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think wrapping these in ifndefs is necessary. Where else would the user define these? Honestly would just leave them there and if someone were to want to change these they can in place.

Copy link
Contributor

Choose a reason for hiding this comment

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

they are needed for the build.py because it will redefine the board.
I also think not the nicest solution.

[env:BOARD_SLIMEVR_V1_2]
platform = espressif8266 @ 4.2.1
board = esp12e
build_flags =
  ${env.build_flags}
  -D BOARD=BOARD_SLIMEVR_V1_2

Maybe nicer would be to have defines.h excluded if we run ci with -D NODEFINES=true ? And disable the include in globals.h?
So it would not affect the avarange diy person or webflasher.

Copy link
Member Author

Choose a reason for hiding this comment

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

It will not affect webflasher anyway, because the guards aren't required unless the defines are set outside of the defines.h. Webflasher modifies the defines.h, so it should work okay. DIY ppl can just ignore them. I think it's a better solution than to repeat all defines in the build flags, that way anything new won't be needed to be added everywhere.

@loucass003
Copy link
Member

Maybe i am picky. But the massive if in the defines for all the gloves defines, i cannot even find the endif from github preview. Maybe we could put that whole thing into its own header?

@Eirenliel Eirenliel merged commit 94f61c7 into main May 23, 2025
2 checks passed
@Eirenliel Eirenliel deleted the ci-new-boards branch May 23, 2025 16:17
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.

5 participants