sascha/esphome
1
0
Fork 0
mirror of https://github.com/esphome/esphome.git synced 2025-10-30 06:33:51 +00:00
Code Issues Releases Wiki Activity

Scheduler fixes (#813)

Browse Source 
* Scheduler fixes

Fixes https://github.com/esphome/issues/issues/789, fixes https://github.com/esphome/issues/issues/788

Also changes to use unique_ptr - this should be much safer than the raw pointers form before (though the scoping rules might cause some issues, but looking closely I didn't find anything)

* Disable debugging

* Format
This commit is contained in:
Otto Winter
2019-10-31 20:25:16 +01:00
parent dde70c95a4
commit 440e428aa4
2 changed files with 112 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files

115
esphome/core/scheduler.cpp
View File

@@ -9,6 +9,9 @@ static const char *TAG = "scheduler";
static const uint32_t SCHEDULER_DONT_RUN = 4294967295UL; static const uint32_t SCHEDULER_DONT_RUN = 4294967295UL;
// Uncomment to debug scheduler
// #define ESPHOME_DEBUG_SCHEDULER
void HOT Scheduler::set_timeout(Component *component, const std::string &name, uint32_t timeout, void HOT Scheduler::set_timeout(Component *component, const std::string &name, uint32_t timeout,
std::function<void()> &&func) { std::function<void()> &&func) {
const uint32_t now = this->millis_(); const uint32_t now = this->millis_();
@@ -21,7 +24,7 @@ void HOT Scheduler::set_timeout(Component *component, const std::string &name, u
ESP_LOGVV(TAG, "set_timeout(name='%s', timeout=%u)", name.c_str(), timeout); ESP_LOGVV(TAG, "set_timeout(name='%s', timeout=%u)", name.c_str(), timeout);
auto *item = new SchedulerItem(); auto item = make_unique<SchedulerItem>();
item->component = component; item->component = component;
item->name = name; item->name = name;
item->type = SchedulerItem::TIMEOUT; item->type = SchedulerItem::TIMEOUT;
@@ -30,7 +33,7 @@ void HOT Scheduler::set_timeout(Component *component, const std::string &name, u
item->last_execution_major = this->millis_major_; item->last_execution_major = this->millis_major_;
item->f = std::move(func); item->f = std::move(func);
item->remove = false; item->remove = false;
this->push_(item); this->push_(std::move(item));
} }
bool HOT Scheduler::cancel_timeout(Component *component, const std::string &name) { bool HOT Scheduler::cancel_timeout(Component *component, const std::string &name) {
return this->cancel_item_(component, name, SchedulerItem::TIMEOUT); return this->cancel_item_(component, name, SchedulerItem::TIMEOUT);
@@ -52,7 +55,7 @@ void HOT Scheduler::set_interval(Component *component, const std::string &name,
ESP_LOGVV(TAG, "set_interval(name='%s', interval=%u, offset=%u)", name.c_str(), interval, offset); ESP_LOGVV(TAG, "set_interval(name='%s', interval=%u, offset=%u)", name.c_str(), interval, offset);
auto *item = new SchedulerItem(); auto item = make_unique<SchedulerItem>();
item->component = component; item->component = component;
item->name = name; item->name = name;
item->type = SchedulerItem::INTERVAL; item->type = SchedulerItem::INTERVAL;
@@ -63,7 +66,7 @@ void HOT Scheduler::set_interval(Component *component, const std::string &name,
item->last_execution_major--; item->last_execution_major--;
item->f = std::move(func); item->f = std::move(func);
item->remove = false; item->remove = false;
this->push_(item); this->push_(std::move(item));
} }
bool HOT Scheduler::cancel_interval(Component *component, const std::string &name) { bool HOT Scheduler::cancel_interval(Component *component, const std::string &name) {
return this->cancel_item_(component, name, SchedulerItem::INTERVAL); return this->cancel_item_(component, name, SchedulerItem::INTERVAL);
@@ -71,7 +74,7 @@ bool HOT Scheduler::cancel_interval(Component *component, const std::string &nam
optional<uint32_t> HOT Scheduler::next_schedule_in() { optional<uint32_t> HOT Scheduler::next_schedule_in() {
if (this->empty_()) if (this->empty_())
return {}; return {};
auto *item = this->items_[0]; auto &item = this->items_[0];
const uint32_t now = this->millis_(); const uint32_t now = this->millis_();
uint32_t next_time = item->last_execution + item->interval; uint32_t next_time = item->last_execution + item->interval;
if (next_time < now) if (next_time < now)
@@ -82,39 +85,43 @@ void ICACHE_RAM_ATTR HOT Scheduler::call() {
const uint32_t now = this->millis_(); const uint32_t now = this->millis_();
this->process_to_add(); this->process_to_add();
// Uncomment for debugging the scheduler: #ifdef ESPHOME_DEBUG_SCHEDULER
static uint32_t last_print = 0;
// if (random_uint32() % 400 == 0) { if (now - last_print > 2000) {
// std::vector<SchedulerItem *> old_items = this->items_; last_print = now;
// ESP_LOGVV(TAG, "Items: count=%u, now=%u", this->items_.size(), now); std::vector<std::unique_ptr<SchedulerItem>> old_items;
// while (!this->empty_()) { ESP_LOGVV(TAG, "Items: count=%u, now=%u", this->items_.size(), now);
// auto *item = this->items_[0]; while (!this->empty_()) {
// const char *type = item->type == SchedulerItem::INTERVAL ? "interval" : "timeout"; auto item = std::move(this->items_[0]);
// ESP_LOGVV(TAG, " %s '%s' interval=%u last_execution=%u (%u) next=%u", const char *type = item->type == SchedulerItem::INTERVAL ? "interval" : "timeout";
// type, item->name.c_str(), item->interval, item->last_execution, item->last_execution_major, ESP_LOGVV(TAG, " %s '%s' interval=%u last_execution=%u (%u) next=%u (%u)", type, item->name.c_str(),
// item->last_execution + item->interval); item->interval, item->last_execution, item->last_execution_major, item->next_execution(),
// this->pop_raw_(); item->next_execution_major());
// }
// ESP_LOGVV(TAG, "\n"); this->pop_raw_();
// this->items_ = old_items; old_items.push_back(std::move(item));
//} }
ESP_LOGVV(TAG, "\n");
this->items_ = std::move(old_items);
}
#endif // ESPHOME_DEBUG_SCHEDULER
while (!this->empty_()) { while (!this->empty_()) {
// use scoping to indicate visibility of `item` variable
{
// Don't copy-by value yet // Don't copy-by value yet
auto *item = this->items_[0]; auto &item = this->items_[0];
if ((now - item->last_execution) < item->interval) if ((now - item->last_execution) < item->interval)
// Not reached timeout yet, done for this call // Not reached timeout yet, done for this call
break; break;
uint8_t major = item->last_execution_major; uint8_t major = item->next_execution_major();
if (item->last_execution > now) if (this->millis_major_ - major > 1)
major++;
if (major != this->millis_major_)
break; break;
// Don't run on failed components // Don't run on failed components
if (item->component != nullptr && item->component->is_failed()) { if (item->component != nullptr && item->component->is_failed()) {
this->pop_raw_(); this->pop_raw_();
delete item;
continue; continue;
} }
@@ -128,6 +135,11 @@ void ICACHE_RAM_ATTR HOT Scheduler::call() {
// - timeouts/intervals get added, potentially invalidating vector pointers // - timeouts/intervals get added, potentially invalidating vector pointers
// - timeouts/intervals get cancelled // - timeouts/intervals get cancelled
item->f(); item->f();
}
{
// new scope, item from before might have been moved in the vector
auto item = std::move(this->items_[0]);
// Only pop after function call, this ensures we were reachable // Only pop after function call, this ensures we were reachable
// during the function call and know if we were cancelled. // during the function call and know if we were cancelled.
@@ -135,7 +147,6 @@ void ICACHE_RAM_ATTR HOT Scheduler::call() {
if (item->remove) { if (item->remove) {
// We were removed/cancelled in the function call, stop // We were removed/cancelled in the function call, stop
delete item;
continue; continue;
} }
@@ -147,33 +158,30 @@ void ICACHE_RAM_ATTR HOT Scheduler::call() {
if (item->last_execution < before) if (item->last_execution < before)
item->last_execution_major++; item->last_execution_major++;
} }
this->push_(item); this->push_(std::move(item));
} else { }
delete item;
} }
} }
this->process_to_add(); this->process_to_add();
} }
void HOT Scheduler::process_to_add() { void HOT Scheduler::process_to_add() {
for (auto *it : this->to_add_) { for (auto &it : this->to_add_) {
if (it->remove) { if (it->remove) {
delete it;
continue; continue;
} }
this->items_.push_back(it); this->items_.push_back(std::move(it));
std::push_heap(this->items_.begin(), this->items_.end(), SchedulerItem::cmp); std::push_heap(this->items_.begin(), this->items_.end(), SchedulerItem::cmp);
} }
this->to_add_.clear(); this->to_add_.clear();
} }
void HOT Scheduler::cleanup_() { void HOT Scheduler::cleanup_() {
while (!this->items_.empty()) { while (!this->items_.empty()) {
auto item = this->items_[0]; auto &item = this->items_[0];
if (!item->remove) if (!item->remove)
return; return;
delete item;
this->pop_raw_(); this->pop_raw_();
} }
} }
@@ -181,15 +189,15 @@ void HOT Scheduler::pop_raw_() {
std::pop_heap(this->items_.begin(), this->items_.end(), SchedulerItem::cmp); std::pop_heap(this->items_.begin(), this->items_.end(), SchedulerItem::cmp);
this->items_.pop_back(); this->items_.pop_back();
} }
void HOT Scheduler::push_(Scheduler::SchedulerItem *item) { this->to_add_.push_back(item); } void HOT Scheduler::push_(std::unique_ptr<Scheduler::SchedulerItem> item) { this->to_add_.push_back(std::move(item)); }
bool HOT Scheduler::cancel_item_(Component *component, const std::string &name, Scheduler::SchedulerItem::Type type) { bool HOT Scheduler::cancel_item_(Component *component, const std::string &name, Scheduler::SchedulerItem::Type type) {
bool ret = false; bool ret = false;
for (auto *it : this->items_) for (auto &it : this->items_)
if (it->component == component && it->name == name && it->type == type) { if (it->component == component && it->name == name && it->type == type) {
it->remove = true; it->remove = true;
ret = true; ret = true;
} }
for (auto *it : this->to_add_) for (auto &it : this->to_add_)
if (it->component == component && it->name == name && it->type == type) { if (it->component == component && it->name == name && it->type == type) {
it->remove = true; it->remove = true;
ret = true; ret = true;
@@ -206,21 +214,30 @@ uint32_t Scheduler::millis_() {
return now; return now;
} }
bool HOT Scheduler::SchedulerItem::cmp(Scheduler::SchedulerItem *a, Scheduler::SchedulerItem *b) { bool HOT Scheduler::SchedulerItem::cmp(const std::unique_ptr<SchedulerItem> &a,
const std::unique_ptr<SchedulerItem> &b) {
// min-heap // min-heap
// return true if *a* will happen after *b* // return true if *a* will happen after *b*
uint32_t a_next_exec = a->last_execution + a->timeout; uint32_t a_next_exec = a->next_execution();
uint8_t a_next_exec_major = a->last_execution_major; uint8_t a_next_exec_major = a->next_execution_major();
if (a_next_exec < a->last_execution) uint32_t b_next_exec = b->next_execution();
a_next_exec_major++; uint8_t b_next_exec_major = b->next_execution_major();
uint32_t b_next_exec = b->last_execution + b->timeout;
uint8_t b_next_exec_major = b->last_execution_major;
if (b_next_exec < b->last_execution)
b_next_exec_major++;
if (a_next_exec_major != b_next_exec_major) { if (a_next_exec_major != b_next_exec_major) {
return a_next_exec_major > b_next_exec_major; // The "major" calculation is quite complicated.
// Basically, we need to check if the major value lies in the future or
//
// Here are some cases to think about:
// Format: a_major,b_major -> expected result (a-b, b-a)
// a=255,b=0 -> false (255, 1)
// a=0,b=1 -> false (255, 1)
// a=1,b=0 -> true (1, 255)
// a=0,b=255 -> true (1, 255)
uint8_t diff1 = a_next_exec_major - b_next_exec_major;
uint8_t diff2 = b_next_exec_major - a_next_exec_major;
return diff1 < diff2;
} }
return a_next_exec > b_next_exec; return a_next_exec > b_next_exec;

18
esphome/core/scheduler.h
View File

@@ -2,6 +2,7 @@
#include "esphome/core/component.h" #include "esphome/core/component.h"
#include <vector> #include <vector>
#include <memory>
namespace esphome { namespace esphome {
@@ -34,21 +35,30 @@ class Scheduler {
bool remove; bool remove;
uint8_t last_execution_major; uint8_t last_execution_major;
static bool cmp(SchedulerItem *a, SchedulerItem *b); inline uint32_t next_execution() { return this->last_execution + this->timeout; }
inline uint8_t next_execution_major() {
uint32_t next_exec = this->next_execution();
uint8_t next_exec_major = this->last_execution_major;
if (next_exec < this->last_execution)
next_exec_major++;
return next_exec_major;
}
static bool cmp(const std::unique_ptr<SchedulerItem> &a, const std::unique_ptr<SchedulerItem> &b);
}; };
uint32_t millis_(); uint32_t millis_();
void cleanup_(); void cleanup_();
void pop_raw_(); void pop_raw_();
void push_(SchedulerItem *item); void push_(std::unique_ptr<SchedulerItem> item);
bool cancel_item_(Component *component, const std::string &name, SchedulerItem::Type type); bool cancel_item_(Component *component, const std::string &name, SchedulerItem::Type type);
bool empty_() { bool empty_() {
this->cleanup_(); this->cleanup_();
return this->items_.empty(); return this->items_.empty();
} }
std::vector<SchedulerItem *> items_; std::vector<std::unique_ptr<SchedulerItem>> items_;
std::vector<SchedulerItem *> to_add_; std::vector<std::unique_ptr<SchedulerItem>> to_add_;
uint32_t last_millis_{0}; uint32_t last_millis_{0};
uint8_t millis_major_{0}; uint8_t millis_major_{0};
}; };