rand256 / valetudo

Valetudo RE - experimental vacuum software, cloud free
Apache License 2.0
667 stars 73 forks source link

last_bin_out #336

Closed vprotsaylo closed 3 years ago

vprotsaylo commented 3 years ago

corrupted last_bin_out value

How to Reproduce

The last_bin_out value is ok when inserting the trash bin container, but it gets corrupted after some time (in less then 24 hour), e.g. it is 2147483647000 which, when deviced by 1000 (miliseconds) is equivalent to: 01/19/2038 @ 3:14am (UTC)

Expected behavior An expected value is UNIX timespamp value multplies by 1000.

Screenshots

image

Vacuum Model: Roborock S5 Valetudo Version: v0.9.9

pidator commented 3 years ago

Do you have checked the timesetting of your robot? Is the linux system time the correct one? https://github.com/rand256/valetudo/blob/2b8e6dfb9cefc380caa3c5951525e99537ef0ed0/lib/MqttClient.js#L358 last_bin_out will be set with the current date...!?

vprotsaylo commented 3 years ago

Just checked the current date and it is correct:

root@rockrobo:~# date
Sat Dec 12 19:11:19 CET 2020
root@rockrobo:~# date +%s
1607796680
rand256 commented 3 years ago

Though the device time is definitely important for this to work, there was an error in the code unfortunately. Thanks for mentioning it.

vprotsaylo commented 3 years ago

Thank you for your time and for the quick fix!

pidator commented 3 years ago

hm, I'm wondering @rand256 if you'll have to add the changes accordingly to the last_bin_full value too or if Date.now() is sufficient here?

https://github.com/rand256/valetudo/blob/5623ee24ec5ad06d429ea103ed0859a3f3dfc098/lib/MqttClient.js#L189

rand256 commented 3 years ago

Well, maybe you're right and we should to save both last_bin_full and last_bin_out, but there's a difference between them:

We get the information about last_bin_out by monitoring some cfg file content by ourselves. If we miss an event when a specific value in the file was reset, we'll never know that the bin was replaced. So we need to store the datetime of such event somewhere so it'd outlive device reboots or valetudo restarts.

In comparison, the information about last_bin_full is generated by firmware as a message it tries to send into the cloud. I guess the firmware would repeat that message as many times as it sees fit until the bin will be actually replaced. So it theory it shouldn't hurt if we just won't care of keeping the datetime upon nightly reboots or service restarts, as we should get that notification again later.