KeepAlive interval of 1s results in panic

[mzjulian]

Hi everyone,

I've noticed, that when an keepAlive interval of 1s <= t < 2s is used, it panics.

I'm using:
opts.SetKeepAlive(1*time.Second)

It produces the following:

non-positive interval for NewTicker
Ticker(0xb739e0)
cal/go/src/time/tick.go:24 +0x10f
om/eclipse/paho%2emqtt%2egolang.keepalive(0xc00018c240, {0x8b1c20, 0xc0000a20d0})
mqtt/vendor/github.com/eclipse/paho.mqtt.golang/ping.go:40 +0x126
by github.com/eclipse/paho%2emqtt%2egolang.(*client).startCommsWorkers
mqtt/vendor/github.com/eclipse/paho.mqtt.golang/client.go:542 +0x325

Is that intended behavior?

my env:
go version:1.17
github.com/eclipse/paho.mqtt.golang v1.3.5

[MattBrittan]

While SetKeepAlive accepts a time.Duration this is converted into seconds internally. When working out when to make the next check it used the following:

	if c.options.KeepAlive > 10 {
		checkInterval = 5
	} else {
		checkInterval = c.options.KeepAlive / 2
	}

	intervalTicker := time.NewTicker(time.Duration(checkInterval * int64(time.Second)))

In your case 1/2 = 0 (integer math) so the call is effectively time.NewTicker(0) which causes the panic.

I suspect that this issue is due to the authors expecting longer keep-alive durations; is there a reason you need to send them this frequently? The spec spec suggests:

The actual value of the Keep Alive is application specific; typically this is a few minutes. The maximum value is 18 hours 12 minutes and 15 seconds.

I'm happy to accept a pull request that resolves this (but don't see this as a priority as sending keep-alives this frequently seems to be of limited use).

[goodwel1]

set keepalive 1 Second 🤪
set keepalive 5 Second 😏

[MattBrittan]

Closing as per PR 531