#495 throttle.lua floor problem

Reporter pierre.gotab
Owner MattJ
Stars ★ (1)  
  • Milestone-0.9
  • Priority-Medium
  • Type-Defect
  • Status-Accepted
  1. pierre.gotab on

    *What steps will reproduce the problem?* 1. th = throttle.create(1, 1) 2. call th:poll(1) more than once per second 3. these poll call will *never* return true *What is the expected output? What do you see instead?* It is expected that poll will finish to return true, whatever the call frequency of :update() *What version of the product are you using? On what operating system?* Any versions on any OSes :) *Please provide any additional information below.* See https://github.com/maranda/metronome/issues/223 for a full description of the problem. And please be smarter than Marco Cirillo :D

  2. MattJ on

    Hi, thanks for the report! I see the problem. However I'm worried it's not so simple. The floor() was not originally there, I added it in this commit: https://hg.prosody.im/trunk/rev/0e9a5b63206a I am quite annoyed at myself for not adding an explanation in a comment or in the commit message, but I'm sure there was a reason I added it. We also don't have any tests for this module, so I'm disinclined to make the obvious change without 1) tests or 2) understanding why I added floor() in the first place. But I agree the current behaviour is certainly wrong. Hmm. I'm setting the milestone as 0.9 because this deserves to be fixed in the stable branch.

    • owner MattJ
    • tags Milestone-0.9 Status-Accepted
  3. pierre.gotab on

    Maybe the floor is there to keep the balance an integer. What I propose to ensure throttle works as intended, and keep balance an integer is: local growth = self.rate * elapsed; if growth >= 1 then local int_growth = floor(growth); self.t = newt - (growth - int_growth) / self.rate; local balance = int_growth + self.balance; if balance > self.max then ... ... -- else dont do anything

  4. MattJ on

    Wouldn't just: - if balance > self.max then + if floor(balance) > self.max then suffice for that?

  5. pierre.gotab on

    No I don't think this condition should be changed. The problem is: b 0 1 |---------|--- t 1 2 ^ :update() call t=2.3 and balance=1.3 ; but keep balance floored so: ^ \_ back in time here t=2 and balance=1 This is the aim of newt - (growth - int_growth) / self.rate = 2.3 - (0.3)/1 = 2 It keeps internal self.t consistent with balance flooring. Like if update was called at t=2 (And obviously if growth < 0 nothing has to be done, just wait for a significant delta time)

  6. pierre.gotab on

    *growth < 1 sorry

New comment