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 broken CI builds on Zephyr 3.2 #6

Merged
merged 5 commits into from
Jul 31, 2023

Conversation

DerDreschner
Copy link

There are several changes in the Zephyr 3.2 branch that break several tests as well as Non-Advantage 360 builds. To ensure a high software quality and ensure being as compatible as possible to upstream, this pull request tries to fix all CI pipelines.

Tests: Commit a981282 broke the backlight tests due to changed default values. The necessary upstream default values are now used for the tests. I also considered changing the tests itself, but I value a high upstream compatibility over a "perfect fitting" repository for the Advantage 360.

Non-Advantage 360 builds: Commit 10b605e broke compatibility with several build configurations that don't use CONFIG_ZMK_SPLIT_BLE, CONFIG_ZMK_RGB_UNDERGLOW and CONFIG_ZMK_BACKLIGHT. I'm not sure if these code changes wouldn't fit better in their respective file inside the behaviour folder. But as first step I were happy with working builds using preprocessor conditions. What's your opinion on that?

For working builds with battery reporting, I needed to cherry-pick 881da25 as well. I guess this change would find their way into the branch in the near future anyway.

@DerDreschner
Copy link
Author

Ahh, I recognized I had to cherry-pick that commit due to the change in 2797cc9. Do you have any experience if enabling deep sleep as proposed here zmkfirmware#1273 (comment) is enough? What are the drawbacks that you decided to remove BT_BAS completely?

And would you be able to use the ZMK_BATTERY_REPORTING setting instead of remove BT_BAS? I'm a Linux user and would really like to get the current battery status without maintaining my own zmk repo in the future.

@DerDreschner
Copy link
Author

DerDreschner commented Jun 26, 2023

Reverted the cherry-pick to keep the changes in this PR non-breaking. Please keep in mind that the nice_nano_v2 kyria_left -DCONFIG_ZMK_DISPLAY=y build is supposed to fail as there are additional code lines in app/src/display/widgets that rely on BT_BAS and being build in that configuration.

@ReFil
Copy link
Owner

ReFil commented Jul 25, 2023

Really sorry i missed this, I didn't get an email. Thank you for the PR, i'll look over it now

@ReFil
Copy link
Owner

ReFil commented Jul 25, 2023

Ahh, I recognized I had to cherry-pick that commit due to the change in 2797cc9. Do you have any experience if enabling deep sleep as proposed here zmkfirmware#1273 (comment) is enough? What are the drawbacks that you decided to remove BT_BAS completely?

And would you be able to use the ZMK_BATTERY_REPORTING setting instead of remove BT_BAS? I'm a Linux user and would really like to get the current battery status without maintaining my own zmk repo in the future.

Yes i'll be going to using the merged configuration option, disabling bas was a stopgap measure.
Enabling deep sleep for everyone runs the risk of causing split connection issues, disabling the battery reporting was a simpler option

@ReFil
Copy link
Owner

ReFil commented Jul 25, 2023

A dedicated bluetooth service to update the lights on the peripheral side is more reliable than the behaviour invocation based technique, and was vital for implementing te layer and hid indicators on the peripheral side

@ReFil
Copy link
Owner

ReFil commented Jul 25, 2023

This looks good, i need to test pulling in the latest upstream then i will merge this in. Thank you very much for your work

@ReFil ReFil merged commit 93bf512 into ReFil:adv360-z3.2 Jul 31, 2023
36 of 37 checks passed
ReFil pushed a commit that referenced this pull request Sep 12, 2023
* fix(backlight/tests): Restore original behaviour to fix backlight tests

* fix(build): Remove feature-specific code when SPLIT_BLE, RGB_UNDERGLOW or BACKLIGHT isn't enabled

* refactor(bluetooth): Add battery reporting config.

* Add dedicated battery reporting Kconfig that is `imply`d by
  enabling ZMK_BLE.

* fix(rgb_underglow): Use correct condition in service.c

* Revert "refactor(bluetooth): Add battery reporting config."

This reverts commit 881da25.

---------

Co-authored-by: Peter Johanson <peter@peterjohanson.com>
ReFil pushed a commit that referenced this pull request Oct 9, 2023
Co-authored-by: Pete Johanson <peter@peterjohanson.com>
ReFil pushed a commit that referenced this pull request Oct 18, 2023
* fix(backlight/tests): Restore original behaviour to fix backlight tests

* fix(build): Remove feature-specific code when SPLIT_BLE, RGB_UNDERGLOW or BACKLIGHT isn't enabled

* refactor(bluetooth): Add battery reporting config.

* Add dedicated battery reporting Kconfig that is `imply`d by
  enabling ZMK_BLE.

* fix(rgb_underglow): Use correct condition in service.c

* Revert "refactor(bluetooth): Add battery reporting config."

This reverts commit 881da25.

---------

Co-authored-by: Peter Johanson <peter@peterjohanson.com>
ReFil pushed a commit that referenced this pull request Oct 30, 2023
* fix(backlight/tests): Restore original behaviour to fix backlight tests

* fix(build): Remove feature-specific code when SPLIT_BLE, RGB_UNDERGLOW or BACKLIGHT isn't enabled

* refactor(bluetooth): Add battery reporting config.

* Add dedicated battery reporting Kconfig that is `imply`d by
  enabling ZMK_BLE.

* fix(rgb_underglow): Use correct condition in service.c

* Revert "refactor(bluetooth): Add battery reporting config."

Co-Authored-By: Peter Johanson <peter@peterjohanson.com>
ReFil pushed a commit that referenced this pull request Dec 17, 2023
* fix(backlight/tests): Restore original behaviour to fix backlight tests

* fix(build): Remove feature-specific code when SPLIT_BLE, RGB_UNDERGLOW or BACKLIGHT isn't enabled

* refactor(bluetooth): Add battery reporting config.

* Add dedicated battery reporting Kconfig that is `imply`d by
  enabling ZMK_BLE.

* fix(rgb_underglow): Use correct condition in service.c

* Revert "refactor(bluetooth): Add battery reporting config."

This reverts commit 881da25.

---------

Co-authored-by: Peter Johanson <peter@peterjohanson.com>
ReFil pushed a commit that referenced this pull request Mar 27, 2024
* fix(backlight/tests): Restore original behaviour to fix backlight tests

* fix(build): Remove feature-specific code when SPLIT_BLE, RGB_UNDERGLOW or BACKLIGHT isn't enabled

* refactor(bluetooth): Add battery reporting config.

* Add dedicated battery reporting Kconfig that is `imply`d by
  enabling ZMK_BLE.

* fix(rgb_underglow): Use correct condition in service.c

* Revert "refactor(bluetooth): Add battery reporting config."

This reverts commit 881da25.

---------

Co-authored-by: Peter Johanson <peter@peterjohanson.com>
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.

3 participants