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

Fix interaction of SO92, VirtualCT, and RGBWWTable #18768

Merged
merged 5 commits into from
Jun 1, 2023

Conversation

jonschz
Copy link
Contributor

@jonschz jonschz commented Jun 1, 2023

Description:

Related issue (if applicable): fixes #15337

Previously, when SO92 (pwm_ct_mode) was enabled, VirtualCT would not work at all, and 5th entry of the RGBWWTable would affect the color temperature instead of the warm white intensity. With this patch, all options interact as intended. I did extensive testing with various modes to ensure that the code works as intended (cw, rgb, rgbw, rgbww with and without SO92, SO105 in all applicable modes, RGBWWTable, all with LEDTable enabled and disabled).

Nothing changes for users who have SO92 disabled.

For those that have SO92 enabled but do not use VirtualCT or RGBWWTable, there are tiny changes to the color temperature when LEDTable is enabled, most notable at low brightness and ct values around 240 (25 %) and around 413 (75 %). If needed, I can provide an argument why I believe the new values to be more correct than the old ones.

This restructure rendered some variables unnecessary, which I hence removed.

Checklist:

  • The pull request is done against the latest development branch
  • Only relevant files were touched
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • The code change is tested and works with Tasmota core ESP8266 V.2.7.4.9 (tested on device)
  • The code change is tested and works with Tasmota core ESP32 V.2.0.9 (tested in simulator)
  • I accept the CLA.

NOTE: The code change must pass CI tests. Your PR cannot be merged unless tests pass

jonschz added 5 commits June 1, 2023 17:58
@jonschz jonschz mentioned this pull request Jun 1, 2023
6 tasks
@s-hadinger
Copy link
Collaborator

Thanks. Let me take a look at it.

@s-hadinger
Copy link
Collaborator

I'm always terrified when changing code in this portion because there are so many corner cases and configuration possibilities. I trust you in testing many different configurations. We can always revert if there are unexpected results.

@jonschz
Copy link
Contributor Author

jonschz commented Jun 1, 2023

I trust you in testing many different configurations

Thank you for that. The change boils down to a different order of operations:

Old: SO92, VirtualCT (if not SO92), RGBWWTable
New: VirtualCT (unconditionally), RGBWWTable, SO92

I can explain the changes in greater detail if needed.

@s-hadinger s-hadinger merged commit 9d57a19 into arendst:development Jun 1, 2023
@s-hadinger
Copy link
Collaborator

Thanks for your contribution!

@jonschz jonschz deleted the so92_fixes branch June 26, 2023 06:09
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.

VirtualCT does not work for RGBCW lights with SetOption92=1
2 participants