blink1 icon indicating copy to clipboard operation
blink1 copied to clipboard

firmware: servertickle maximum delay

Open phi1010 opened this issue 8 years ago • 2 comments

--servertickle -t $num seems to have a quite low limit for $num, somewhere between 65 and 66 seconds, probably at 65536ms. This makes it difficult to use a cronjob (not running more often than once a minute) when the test verifying correct functionality takes a few seconds, sometimes more, sometimes less, varying more than 6 seconds between runs.

As far as i can see, that problem is caused by dividing by 10 after converting the parameter parsed as long int to uint16_t, causing the maximum possible value to be 655310ms, instead of 6553610ms, as probably intended.

//commandline/blink1-lib.c
int blink1_serverdown(blink1_device *dev, uint8_t on, uint16_t millis, uint8_t st)
{
    int dms = millis/10;  // millis_divided_by_10
    // ...
    buf[3] = (dms>>8);
    buf[4] = (dms % 0xff);
    // ...
}

phi1010 avatar Apr 25 '16 04:04 phi1010

Hi Phil, You're right. There's a few dumb mistakes in there. I'll get a fix checked in tomorrow for it. Thanks for catching that.

todbot avatar Apr 25 '16 07:04 todbot

After doing testing and fixing some int casting issues in blink1-lib, it looks like the real limitation is in the blink(1) firmware. (looks like a casting bug there that too)

Therefore, the effective max range of the "serverdown" / "servertickle" feature is 65 seconds (e.g. blink1-tool -t 65000 --servertickle 1)

The original use case for this feature was to have a simple persistent daemon repeatedly calling servertickle while doing something that could potentially block, like:

while [ 1 ] ; do 
  blink1-tool -t 2000 --servertickle 1
  # do something else
  sleep 1
done

I didn't anticipate its use directly in a cronjob. Apologies if this caused you problems, and thanks for finding it. In a month or so I'm going to be revisiting the firmware for our next production run and I'll see about fixing this problem when I'm in there.

todbot avatar Apr 27 '16 05:04 todbot