Fix brightness calculation when using brightness_step_pct

[andreaskoelsch]

Proposed change

Fixed brightness calculation when using percentage steps. Before, occasionally, rounding error could occur. The issue is described in #143559.

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Deprecation (breaking change to happen in the future)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes light.turn_on brightness_step_pct skips values #143559
• This PR is related to issue:
• Link to documentation pull request:
• Link to developer documentation pull request:
• Link to frontend pull request:

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• I have followed the perfect PR recommendations
• The code has been formatted using Ruff (ruff format homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[home-assistant]

Hi @andreaskoelsch

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

[home-assistant]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

[home-assistant]

Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration (light) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of light can trigger bot actions by commenting:

• @home-assistant close Closes the pull request.
• @home-assistant rename Awesome new title Renames the pull request.
• @home-assistant reopen Reopen the pull request.
• @home-assistant unassign light Removes the current integration label and assignees on the pull request, add the integration domain after the command.
• @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
• @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

Stale

[abmantis]

Could you also add/update the tests to cover the case that was causing the issue (the 1% increments)?

[andreaskoelsch]

I will look into it. Might take a week or two till I find time for it.

[MartinHjelmare]

It's normally not a good practice to round before the final result. It's not clear why that is appropriate here. I've read the issue.

[jbouwh]

May be use the helpers?

core/homeassistant/util/color.py

Lines 752 to 784 in 9802167

	def brightness_to_value(low_high_range: tuple[float, float], brightness: int) -> float:
	"""Given a brightness_scale convert a brightness to a single value.
	Do not include 0 if the light is off for value 0.
	Given a brightness low_high_range of (1,100) this function
	will return:
	255: 100.0
	127: ~49.8039
	10: ~3.9216
	"""
	return scale_to_ranged_value((1, 255), low_high_range, brightness)
	def value_to_brightness(low_high_range: tuple[float, float], value: float) -> int:
	"""Given a brightness_scale convert a single value to a brightness.
	Do not include 0 if the light is off for value 0.
	Given a brightness low_high_range of (1,100) this function
	will return:
	100: 255
	50: 128
	4: 10
	The value will be clamped between 1..255 to ensure valid value.
	"""
	return min(
	255,
	max(1, round(scale_to_ranged_value(low_high_range, (1, 255), value))),
	)

[andreaskoelsch]

It's normally not a good practice to round before the final result. It's not clear why that is appropriate here. I've read the issue.

The problem is that the brightness is saved as [0, 255] value, and when you trigger a percentage-based brightness step, there can be a rounding error, which can accumulate if you do it multiple times. To not accumulate this rounding error, the percentage based steps should happen in the percentage domain instead of the brightness domain, i.e. the brightness has to be converted to the percentage domain (first rounding, equalize the potential rounding error from before), then do the stepping, then convert back to brightness domain (second rounding).

May be use the helpers?

Ohh, didn't see them as they were not used before either. Also, I don't think it would improve readability, as it's consistently not used in the file (cf.

core/homeassistant/components/light/__init__.py

Line 347 in b658371

params[ATTR_BRIGHTNESS] = round(255 * brightness_pct / 100)

). However, if you wish, I can of course change it.

EDIT: Also worth noting, this rounding error accumulation does not only happen for small steps, but also for larger steps.

• 3 * 1% = 4%
• 3 * 10% = 31%
• 3 * 30% = 91%

[andreaskoelsch]

I tried to set up an env for writing tests on a Ubuntu VM, but I'm stuck here (#142304). Guess, I'll wait for a fix for this issue before I continue working on this PR.

EDIT: Managed to get it running using a dev container

[andreaskoelsch]

Added a test that reduces the brightness 3 times by 10% starting from 100%. The expected result is 70%. Without the proposed code change, this test will fail, because brightness would end up at 69%.

[abmantis]

One small change, otherwise LGTM

[abmantis]

Thanks @andreaskoelsch !