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

process Backlog commands without minimum delay #6562

Merged
merged 2 commits into from
Oct 9, 2019

Conversation

ljakob
Copy link
Contributor

@ljakob ljakob commented Oct 4, 2019

Description:

improve latency processing rules with multiple commands via Backlog

Checklist:

  • The pull request is done against the latest dev branch
  • Only relevant files were touched
  • Only one feature/fix was added per PR.
  • tested on v6.6.0
  • The code change pass travis tests. Your PR cannot be merged unless tests pass
  • I accept the CLA.

@Jason2866
Copy link
Collaborator

This brakes backward compability and for some commands a delay is necessary (rf bridge)

@ljakob
Copy link
Contributor Author

ljakob commented Oct 4, 2019

This brakes backward compability and for some commands a delay is necessary (rf bridge)

what do you propose? I've no problem keeping that patch locally. I needed a solution where rules with a couple of commands are processed without noticeable delay.

@Jason2866
Copy link
Collaborator

The idea for the possibility to have no delay in between backlogs commands is fine.
For a 100% solution it needs to behave like before and with a (new) Setoption to change to minimum delay.
Anyway, It is up to Theo to decide!

@andrethomas
Copy link
Contributor

Perhaps this can be set to a default of 2 and user modified from my_user_config.h

I don't think it warrants wasting a setoption location for something that would not necessarily need to be changed during runtime - I think being able to set this prior to compile would be sufficient.

@ascillato
Copy link
Contributor

Agree

@laurentdong
Copy link
Contributor

I like the idea that have an adjustable Setoption for backlog delay time. It would be better to have the setting by ms instead of 100ms. In order to keep the backward compatibility, the default value could be 200.

@arendst arendst self-assigned this Oct 9, 2019
@arendst
Copy link
Owner

arendst commented Oct 9, 2019

Phase 1

@arendst arendst merged commit cfe3014 into arendst:development Oct 9, 2019
arendst added a commit that referenced this pull request Oct 9, 2019
Add command SetOption34 0..255 to set backlog delay. Default value is 200 (mSeconds) (#6562)
@arendst
Copy link
Owner

arendst commented Oct 9, 2019

And phase 2.

Everybody happy with command SetOption34 0..255 to set backlog delay. Default stayed at 200 mSec

@arendst arendst removed their assignment Oct 9, 2019
@andrethomas
Copy link
Contributor

@arendst You're the only one that HAS to be happy but I am sure others are also :)

@Jason2866
Copy link
Collaborator

Happiness all around 😀
Thx!

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.

6 participants