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

[tuyamcu_v2] Fix suppressed dimmer updates from MQTT #20950

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

marcusb
Copy link
Contributor

@marcusb marcusb commented Mar 14, 2024

Description:

The driver tried to avoid loops when state updates from the MCU (eg
from physical button press) could be reflected back by Tasmota and
trigger another MCU command, followed by a state update. It did this
by tracking the source of the command in the last_source and
last_command_source variables, suppressing the command if either of
those was SRC_SWITCH.

However this logic is faulty: Since there are two last_source
variables to check, a command might reset one of them, but the other
would still suppress the update. As it turns out, MQTT commands would
only set last_source but not last_command_source. As a result, any
dimmer changes via MQTT would be dropped by the driver and not applied
to the MCU.

Switch functionality (on/off) was still working because those do not
rely on last_command_source, only last_source.

This change removes the loop detection logic altogether for dimmer
updates. This should be safe, because the driver already has the
latest dimmer value in its shadow state, and will not try to re-apply
a current value, thus breaking the loop.

This patch has been tested with several CE-WF500D dimmers which had
this problem.

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.6
  • The code change is tested and works with Tasmota core ESP32 V.2.0.14
  • I accept the CLA.

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

@marcusb marcusb changed the title Ignore cmd source [tuyamcu_v2] Fix suppressed dimmer updates from MQTT Mar 14, 2024
@marcusb
Copy link
Contributor Author

marcusb commented Mar 14, 2024

cc: @btsimonh

The driver tried to avoid loops when state updates from the MCU (eg
from physical button press) could be reflected back by Tasmota and
trigger another MCU command, followed by a state update. It did this
by tracking the source of the command in the last_source and
last_command_source variables, suppressing the command if either of
those was SRC_SWITCH.

However this logic is faulty: Since there are two last_source
variables to check, a command might reset one of them, but the other
would still suppress the update. As it turns out, MQTT commands would
only set last_source but not last_command_source. As a result, any
dimmer changes via MQTT would be dropped by the driver and not applied
to the MCU.

Switch functionality (on/off) was still working because those do not
rely on last_command_source, only last_source.

This change removes the loop detection logic altogether for dimmer
updates. This should be safe, because the driver already has the
latest dimmer value in its shadow state, and will not try to re-apply
a current value, thus breaking the loop.

This patch has been tested with several CE-WF500D dimmers which had
this problem.
@arendst arendst merged commit 1a462c9 into arendst:development Mar 14, 2024
64 checks passed
@marcusb marcusb deleted the ignore-cmd-source branch March 14, 2024 17:00
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.

2 participants