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

Add support for backlog with no delay #9544

Closed
wants to merge 1 commit into from

Conversation

emontnemery
Copy link
Contributor

@emontnemery emontnemery commented Oct 14, 2020

Description:

Add support for executing backlog with no delay without setting SetOption34 = 0

The delay defined by SetOption34 is omitted for any command in a backlog sequence following immediately after the special command NoDelay.
This must be used with care, and only for simple commands.

The main driver is to allow HomeAssistant to more precisely control lights, in particular transitions (Fade and Speed in Tasmota).

Example:

Disable fade, and turn off a light without any delays:
Backlog NoDelay;Fade 0;NoDelay;Power4 OFF
Enable fade, and turn on a light without any additional delays:
Backlog NoDelay;Fade 1;NoDelay;Speed 4;NoDelay;Power4 ON

Checklist:

  • The pull request is done against the latest dev branch
  • Only relevant files were touched
  • Only one feature/fix was added per PR.
  • The code change is tested and works on Tasmota core ESP8266 V.2.7.4.3
  • The code change is tested and works on core ESP32 V.1.12.2
  • I accept the CLA.

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

@arendst
Copy link
Owner

arendst commented Oct 15, 2020

I like the idea but would like to play with backlog_delay value in order to execute the next command removing the code execution redundancy.

Let me have a look before optionally merging this.

@arendst arendst self-assigned this Oct 15, 2020
@arendst arendst added the on hold by dev team Result - Feature request put on hold by member of development team label Oct 15, 2020
arendst added a commit that referenced this pull request Oct 15, 2020
Add command ``NoDelay`` for immediate backlog command execution by Erik Montnemery (#9544)
@arendst
Copy link
Owner

arendst commented Oct 15, 2020

Thx. Implemented my way.

@arendst arendst closed this Oct 15, 2020
@arendst arendst removed the on hold by dev team Result - Feature request put on hold by member of development team label Oct 15, 2020
@emontnemery
Copy link
Contributor Author

emontnemery commented Oct 15, 2020

@arendst I first implemented your way, because as you mention code is much cleaner.

However, it doesn't work because with this way there is a delay between handling of the Backlog command itself and until BacklogLoop starts processing the first command in the Backlog.

What could work, while still keeping the NoDelay handling only in BacklogLoop, is to allow BacklogLoop to check if the next command is NoDelay.

What I mean is: revert 29e73da and instead do something like this:

void BacklogLoop(void) {
  if (TimeReached(backlog_delay) || NextCommandNoDelay()) {

Let me know if you want me to give it a try?

@arendst
Copy link
Owner

arendst commented Oct 15, 2020

@emontnemery How about leaving the commit and add the following to support_command.ino:


    }
//    ResponseCmndChar(D_JSON_APPENDED);
    mqtt_data[0] = '\0';
    backlog_delay = 0;
  } else {

So add line 347 with backlog_delay = 0;

@emontnemery
Copy link
Contributor Author

emontnemery commented Oct 15, 2020

@arendst that is the perfect solution if the delay between the backlog command itself and the first command in the backlog is not needed!

By the way, I think this line in your solution also needs to be changed, to handle if user ends the backlog sequence with NoDelay?

-      } while (nodelay_detected);
+      } while (!BACKLOG_EMPTY && nodelay_detected);

Edit: OK, so it's fixed in 074aec0, great!

@ascillato2 ascillato2 added hacktoberfest-accepted Type - Issue approved for Hacktoberfest Challenge and removed hacktoberfest-accepted Type - Issue approved for Hacktoberfest Challenge labels Oct 21, 2020
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.

4 participants