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

Extend PWM resolution from 8 to 10 bits for low brightness lights. #5742

Merged
merged 7 commits into from
May 4, 2019

Conversation

s-hadinger
Copy link
Collaborator

Description:

The internal ESP8266 PWM implementation is 10 bits resolution but we use only 8 for LED control. This makes a significant difference when activating Gamma Correction (Ledtable 1) since the Gamma curve is very flat at low light levels. Going to 10 bits resolution makes a smoother low light display and better color accuracy at low brightness.

Due to current implementation of PWM, it seems that you cannot go beyond 9 bits resolution with core_2_3_0 or core_2_4_2. Full range is available when switching to core_2_5_0 and when PWMFrequency is below 440 Hz.

To see the difference:

Activate Gamma Correction: LedTable 1
Set color to ligth white: Color #FFFFFF
Set Dimmer to 1: Dimmer 1
Set PMW Frequency to 440 Hz: PwmFrequency 440

With previous version (8 bits), you won't see an increase in brightness from Dimmer 1 until Dimmer 13.

With current version with core_2_5_0, you'll see a first light increase at Dimmer 8, then at Dimmer 11 and so on.

**Related issue (if applicable):

Implementation:

  • I internally use a new adaptative resolution Gamma table that uses the same memory footprint as the original table (256 bytes) - hence all table values are below 255. To ease calculation, the table goes from 11 bits resolution in low light up to 8 bits. Gamma correction works no more by looking the Gamma table but calling ledGamma() instead.
  • Ability to use full power LEDs. Previously, max PWM value was 1020 (255*4), and was limited to 1008 because of issue Command "Dimmer 100" breaks H801 device #1146. Although it does not make a visual difference, PMW sometimes creates a small buzzing noise in LEDs because of internal vibration. Currently, a LED value of 255 actually drives the PMW to 1023 (max). I patched the 2_3_0 and 2_4_2 to detect a PMW==1023 value and deactivate PWM in this case (this is native behavior in 2_5_0). This means that when LED is off or LED is at full power, there are no more vibrations or noises.
  • Added a formal light_subtype LST_MAX value (currently 5) to size all arrays. Also added a check to ensure that light_subtype is never above LST_MAX. If this would accidentally happen, the code would crash because of arrays overflows.
  • Added detailed documentation about the flow of calculations from changing a color value to driving the PWM pins.
  • Code increase: 352 bytes (core_2_5_0)

Checklist:

  • The pull request is done against the latest dev branch
  • Only relevant files were touched (Also remember to update changelog.ino file)
  • Only one feature/fix was added per PR.
  • The code change is tested and works.
  • The code change pass travis tests. Your PR cannot be merged unless tests pass
  • I accept the CLA.

@s-hadinger
Copy link
Collaborator Author

Yes, the patch is to retrofit a native behavior in 2.5.0. There is no change required in 2.5.0.

The patch is to make sure that a PMW value of 1023 just forces the PIN to HIGH value and deactivates PWM. It was already done for a value of zero.

@s-hadinger
Copy link
Collaborator Author

I forgot to mention in the PR description that it only changes behavior for direct PMW pins.

It has no impact on devices with their own PMW like: SM16716, Tuya, Armtronix, PS 16 DZ.

@arendst arendst merged commit 472ac7c into arendst:development May 4, 2019
@s-hadinger
Copy link
Collaborator Author

Thanks.
@arendst Sorry, I forgot to add the description in _changelog.ino.

Could you please add the following or should I do a PR for it?
* Extend PWM resolution from 8 to 10 bits for low brightness lights

@arendst
Copy link
Owner

arendst commented May 4, 2019

I'm mostly offline. Just add it with your next PR 😀

@s-hadinger s-hadinger deleted the feature/led_gamma_10_bits branch September 10, 2019 08:47
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