Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 21, 2025

Plan to fix brightness percentage off-by-one bug

  • Update percentageWithinRange function in src/utils.ts to use standard percentage formula (0-100)
  • Remove the special case for 0% in setBrightnessPercentage function in src/driver.ts
  • Update tests in tests/utils.test.ts to reflect new expected behavior
  • Update tests in tests/driver.test.ts for setBrightnessPercentage to reflect new expected values
  • Run all tests to verify the fix works correctly
  • Build the project to ensure no compilation errors
  • Request code review and run security checks
  • Address code review feedback: Remove redundant Math.round

Summary

Successfully fixed the off-by-one bug where 0% and 1% brightness produced the same value (minimum brightness).

Changes:

  1. Updated percentageWithinRange formula: ((percentage - 1) / (100 - 1))(percentage / 100)
  2. Removed special case for 0% in setBrightnessPercentage
  3. Removed redundant Math.round (Math.ceil already returns integers)
  4. Updated tests and added verification test for 0% vs 1%

Verification:

  • ✅ All 76 tests pass
  • ✅ Build successful
  • ✅ Linter passed
  • ✅ Code review: Feedback addressed
  • ✅ Security scan: No vulnerabilities

Behavior (Litra Glow 20-250 lumen):

  • 0% → 20 lumen (exact minimum) ✓
  • 1% → 23 lumen (above minimum) ✓
  • 50% → 135 lumen (midpoint) ✓
  • 100% → 250 lumen (exact maximum) ✓

Fixes #556

Original prompt

Problem

As reported in issue #556, there's an off-by-one bug in the brightness percentage calculation where both 0% and 1% brightness return the same value (the minimum brightness).

Current Behavior

In src/driver.ts, the setBrightnessPercentage function has a special case for 0% that returns minimumBrightness, but the percentageWithinRange utility function in src/utils.ts also maps 1% to the minimum brightness due to its formula:

// In utils.ts
export const percentageWithinRange = (
  percentage: number,
  startRange: number,
  endRange: number,
): number => {
  return Math.round(
    Math.ceil(((percentage - 1) / (100 - 1)) * (endRange - startRange) + startRange),
  );
};

For a Litra Glow (min: 20 lumen, max: 250 lumen):

  • 0% → 20 lumen (special case)
  • 1% → ((1-1)/(100-1)) * (250-20) + 20 = 20 lumen
  • 100% → 250 lumen

Both 0% and 1% produce identical results (20 lumen), which is incorrect.

Expected Behavior

Each percentage value from 0-100 should map to a unique brightness value within the device's supported range:

  • 0% → minimum brightness (20 lumen for Litra Glow)
  • 1% → slightly above minimum
  • 100% → maximum brightness (250 lumen for Litra Glow)

Suggested Fix

  1. Update the percentageWithinRange function in src/utils.ts to use the standard percentage formula that maps 0-100 to the full range:
export const percentageWithinRange = (
  percentage: number,
  startRange: number,
  endRange: number,
): number => {
  return Math.round(
    Math.ceil((percentage / 100) * (endRange - startRange) + startRange),
  );
};
  1. Remove the special case for 0% in setBrightnessPercentage in src/driver.ts since the updated formula will correctly handle it:
return setBrightnessInLumen(
  device,
  percentageWithinRange(brightnessPercentage, minimumBrightness, maximumBrightness),
);
  1. Update any existing tests in the test files to reflect the new expected behavior.

Fixes #556

This pull request was created from Copilot chat.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits December 21, 2025 09:29
- Updated percentageWithinRange function to use standard 0-100 percentage formula
- Removed special case for 0% in setBrightnessPercentage
- Updated tests to reflect new expected behavior
- Added test to verify 0% and 1% produce different brightness values

Co-authored-by: timrogers <[email protected]>
Copilot AI changed the title [WIP] Fix off-by-one bug in brightness percentage calculation Fix off-by-one error in brightness percentage calculation Dec 21, 2025
Copilot AI requested a review from timrogers December 21, 2025 09:42
@timrogers timrogers requested review from Copilot and removed request for timrogers December 21, 2025 09:59
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an off-by-one error where both 0% and 1% brightness incorrectly mapped to the same lumen value (the device minimum). The fix updates the percentage calculation formula to use standard percentage math and removes the now-unnecessary special case handling.

Key Changes:

  • Updated percentageWithinRange formula from (p-1)/99 to p/100 to correctly map the full 0-100% range
  • Removed redundant 0% special case in setBrightnessPercentage
  • Added test to verify 0% and 1% produce distinct brightness values

Reviewed changes

Copilot reviewed 4 out of 14 changed files in this pull request and generated 1 comment.

File Description
src/utils.ts Updated percentage formula to use standard p/100 calculation
src/driver.ts Removed special case handling for 0% brightness
tests/utils.test.ts Updated test expectations to reflect new formula behavior
tests/driver.test.ts Added test verifying 0% and 1% produce distinct brightness values

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@timrogers
Copy link
Owner

@copilot Is it implemented so that values are properly distributed and:

0% returns the exact minimum
100% returns the exact maximum
1% and other small percentages always return values above the minimum
The midpoint (50%) remains approximately the same?

Math.ceil always returns an integer, making the outer Math.round redundant.
This simplifies the code while maintaining the same behavior.

Co-authored-by: timrogers <[email protected]>
Copy link
Contributor Author

Copilot AI commented Dec 21, 2025

@copilot Is it implemented so that values are properly distributed and:

0% returns the exact minimum...

Yes, the implementation correctly:

  • 0% returns exact minimum (20 for Litra Glow)
  • 100% returns exact maximum (250 for Litra Glow)
  • 1% and other small percentages return values above minimum (1% = 23 lumen)
  • 50% remains at midpoint (135 lumen, which is the midpoint of the 20-250 range)

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 14 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@timrogers
Copy link
Owner

Fixes #556.

@timrogers timrogers changed the title Fix off-by-one error in brightness percentage calculation fix: fix litra-brightness command and setBrightnessPercentage function so 0% and 1% are distinct Dec 21, 2025
@timrogers timrogers marked this pull request as ready for review December 21, 2025 20:36
@timrogers timrogers merged commit ef183a4 into main Dec 21, 2025
3 checks passed
@github-actions
Copy link

🎉 This PR is included in version 4.5.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Brightness percentage inquiry

2 participants