Smartsleep implementation

[martinhjelmare]

I'm currently testing the smartSleep method from dev branch together with pin change interrupt. I think we should look at changing the order of calls, unless we care to miss events, button presses for example.

The problem I've experienced is that after waking up from a pin change, reading the pin state is not done fast enough, because the node is waiting for the smartSleep duration after the heartbeat. So the pin might have gone back to the old state when the node gets to reading it.

Right now the order of calls in smartSleep is like this:

sleep
send heartbeat
wait for duration

I suggest to change to:

send heartbeat
wait for duration
sleep

This way, the node won't miss events waking up. Care still has to be taken to not call smartSleep when you expect a pin change event. For a momentary push button, for example, you should not call smartSleep until the button is reset to its original position, ie pin==HIGH for mode INPUT_PULLUP. Until that has happened you can call regular sleep instead.

I've tested my proposed change for the described scenario and it works better than the previous implementation.

What do you think?

[hek]

Sounds good! Please create an PR.

[abmantis]

I have made a PR for that: https://github.com/mysensors/Arduino/pull/465
By the way, I think the default MY_SMART_SLEEP_WAIT_DURATION is too high (500ms). Using MYSController, a reboot command is received using only 100ms. I've not tried a lower value yet.

Also, couldn't we implement a smarter way to do this? Instead of just waiting and listening for the reboot command (or any other), the node could ask the GW if there's any command pending. The GW would always reply, and the node would sleep right after the reply (or execute the command if requested). That way, there is no need to wait when there are no commands.

[hek]

Yes the wait-duration is set a bit high by default to account for relayed and potential signed messages. In a small un-signed network a lower values is probably ok (if you have a responsive controller).

Are you proposing a special message controller sends when all buffered messages has been transmitted? We still would need some timeout when node should go back to sleep in case this message gets lost or if controller misses the wake up message from the node.

[abmantis]

Yes, the node could send the heartbeat and wait for a "no commands" message from the GW. We could have a timeout like we have now, but most of the times the node would be sleeping much sooner.

[hek]

Sounds good to me.

Will you create an issue or do the implementation?

[abmantis]

I made an issue: https://github.com/mysensors/Arduino/issues/468

I would gladly do the implementation, but I won't have enough time to work on it soon :/

[martinhjelmare]

@abmantis

Hi!

When I tested the changed order of calls in smartSleep method, I noticed that the heartbeat counter was increased with multiples of three, instead of one. Did you notice the same thing?

[abmantis]

@martinhjelmare Yeah I did noticed that! But since I've never seen any heartbeat messages before, I tough that it was normal and didn't though that it was related to the change.

[martinhjelmare]

@abmantis

It seems related, but I haven't looked deeper into why. I guess this ought to be addressed in the PR so that the counter is increased +1 always. But we should find the cause first.