Open konimaru opened 1 year ago
I'd say the usual way to write this is:
Your logic is sound but to avoid the subtraction and for clarity I would propose something like:
const auto timeNow = std::chrono::high_resolution_clock::now();
- const auto triggerExpired = timeNow + std::chrono::seconds(TRIGGER_TIMEOUT_SECONDS);
+ const auto triggerInterval = std::chrono::seconds(TRIGGER_TIMEOUT_SECONDS);
//check trigger for periodic search
- if (this->periodicSearchEnabled and data->lastTimeSearch > triggerExpired)
+ if (this->periodicSearchEnabled and data->lastTimeSearch + triggerInterval <= timeNow)
//check trigger for periodic notify
- if (this->periodicNotifyEnabled and data->lastTimeNotify > triggerExpired)
+ if (this->periodicNotifyEnabled and data->lastTimeNotify + triggerInterval <= timeNow)
We are on the same page then. Admittedly, my proposed version is harder to read/understand but was the quickest way to get it working (minimal change).
How is this handled here, pull request as usual?
A PR is fine.
(and the reasoning why I dislike the substraction is that high_resolution_clock might be a steady_clock which in turn might have unsigned overflow, maybe if this is run in the first 60 seconds after a reboot)
This is just an observation, feel free to convince me that I misunderstood something :)
Current behaviour:
This never works, I only ever get one notification once the server is started, then never again.
Proposed change:
With that I do get periodic notifications.