Fix AVR backlight breathing: low brightness limit & exceeding breathing table max index

[David-customMK]

In attempting to do simple backlight breathing on Atmega32U4, I ran into two problems:

• The backlight was limited to a very low value during breathing. Adjusting brightness manually would briefly flash a higher brightness, then immediately resume the low brightness level. A demonstration of this behavior can be seen here: https://youtube.com/shorts/b1qZkLxvO1U?feature=share
• The backlight would gradually turn on, then off, and then gradually turn on again a little bit before restarting the cycle from off. The expected behavior was for it to not turn back on before restarting a breathing cycle

Description

Investigating the first problem of low backlight limit, it appears that the data pulled from the breathing table was cast to uint16_t before being multiplied to ICRx (line 415 of backlight_avr.c). This limits the multiplication result to a uint16_t value, which is then divided by 255. This limits the overall result of the calculation to 257, which is artificially low (since scale_backlight() accepts uint16_t as input) . In other places like line 289 of backlight_avr.c where a similar calculation is performed, the multiplicand for ICRx is cast to a uint32_t.

Casting to uint32_t in line 415 (now line 419) has been tested and performs backlight breathing correctly, going all the way up to the assigned brightness level.

I notice similar multiplication operations being performed in backlight_chibios.c, line 150, but I have not tested on ARM yet to determine if it is a problem or not. I am not sure, perhaps it is compiler-dependent.

---

Regarding the problem of backlight briefly turning on between breathing cycles, it appears that the modulo operation performed on line 408 in backlight_avr.c is not ideal because of truncation on the interval variable. For example:

• a breathing period of 6 seconds results in an interval calculation on line 405 of 6 * 120 / 128 = 5.625, which is then truncated to interval = 5.
• breathing_counter on line 407 is then limited to 6 * 120 = 720.
• On line 408, because 5.625 was truncated to 5 earlier, breathing_counter / interval can give results as high as 720 / 5 = 144. Then doing a modulo operation on BREATHING_STEPS of 128 results in progressing 1/8th of the way through a breathing cycle again.

It seems much cleaner to skip the modulo operation, and instead just limit index to the maximum valid value for index which is BREATHING_STEPS - 1.

I checked backlight_chibios.c for the same issue and the problem does not exist there, because a breathing_ISR_frequency of 256 Hz is used instead of 120 Hz. Thus, for 128 steps in the breathing table, the math period * 256 / 128 will always result in an exact interval. So technically, the % BREATHING_STEPS modulo operation on line 149 of backlight_chibios.c can be removed, as long as BREATHING_STEPS = 128 (but unlike the changes in this pull request, I haven't tested removal of the chibios modulo on actual hardware)

It should be noted that another possible solution for the AVR code might be also adopt a 128 or 256 Hz breathing ISR frequency, but I've not tested that approach given the ominous comment Only run this ISR at ~120 Hz on line 399.

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Issues Fixed or Closed by This PR

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).