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

Improve Advantage360 battery handling code #17

Merged
merged 1 commit into from
Apr 8, 2024
Merged

Conversation

khoek
Copy link

@khoek khoek commented Apr 4, 2024

The current battery LED handling code has a couple of idiosyncrasies:

  • It has a bug where if the battery state of charge is exactly 80 then all of the indicator LEDs display red.
  • It does not respect the CONFIG_ZMK_RGB_UNDERGLOW_BRT_SCALE like the rest of the code in rgb_underglow.c does (e.g. even the red right-module disconnected flashes), and so the LEDs always come on with full brightness.

I've added a routine which computes the correctly scaled brightness and simplified the code to use it; now there is a list of colors and thresholds which are easily configurable.

Let me know if you would like any changes! Happy to contribute (tested on a real Adv360 Pro).

@ReFil
Copy link
Owner

ReFil commented Apr 5, 2024

Hi, thank you so much for the work you've done on these PRs! They look really well done and refresh a lot of the old code that hasn't been changed since the 360 pro was released. Thanks for targeting the latest Zephyr 3.5 branch too, as it will be good to get these changes in before releasing that update.

@khoek
Copy link
Author

khoek commented Apr 5, 2024

No worries at all! Will-do on the config PRs for the others. BTW the workflow had a failure, but it looks spurious to me---or if not, are you able to tell from the logs there what the issue was (all I see is a non-terminal warning and then cmake exiting with code 1)?

@khoek
Copy link
Author

khoek commented Apr 6, 2024

Sorry for the delay sorting out the build failure, I had to get a ZMK dev environment set up. Building corne_right indeed does result in the compilation error

/home/khoek/code/zmk/app/src/split/bluetooth/service.c:54:41: error: storage size of 'update_led_data' isn't known
   54 | static struct zmk_split_update_led_data update_led_data;
      |                                         ^~~~~~~~~~~~~~~
compilation terminated due to -Wfatal-errors.

but I can pretty confidently say that that wasn't my doing! So evidently one of the shields is broken due to I guess some #include changing somewhere.

@ReFil
Copy link
Owner

ReFil commented Apr 7, 2024

Thanks for looking into it, yeah it's probably some include somewhere, there have been issues with that before too. I might rework the build workflows to just build the 360 pro halves at some point. So long as most of trhe build workflowsa and pre-commit checks pass that's good with me

@ReFil
Copy link
Owner

ReFil commented Apr 8, 2024

All tested, lgtm

@ReFil ReFil merged commit 1dc601e into ReFil:adv360-z3.5 Apr 8, 2024
39 of 41 checks passed
@ReFil
Copy link
Owner

ReFil commented Apr 8, 2024

Fixed the CI failure too, #6 got slightly muddled as i was rebasing to the latest ZMK, should all be sorted now

@khoek khoek deleted the k branch April 10, 2024 01:16
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