Fix sensor filters using C++ undefined behavior (#293)

* Attempt fix for sensor filter UB

* Fix

src/esphomelib/sensor/filter.cpp

@@ -14,7 +14,7 @@ namespace sensor {
 
 SlidingWindowMovingAverageFilter::SlidingWindowMovingAverageFilter(size_t window_size, size_t send_every, size_t send_first_at)
     : send_every_(send_every), send_at_(send_every - send_first_at),
-      value_average_(SlidingWindowMovingAverage(window_size)) {
+      average_(SlidingWindowMovingAverage(window_size)) {
 
 }
 size_t SlidingWindowMovingAverageFilter::get_send_every() const {
@@ -24,13 +24,13 @@ void SlidingWindowMovingAverageFilter::set_send_every(size_t send_every) {
   this->send_every_ = send_every;
 }
 size_t SlidingWindowMovingAverageFilter::get_window_size() const {
-  return this->value_average_.get_max_size();
+  return this->average_.get_max_size();
 }
 void SlidingWindowMovingAverageFilter::set_window_size(size_t window_size) {
-  this->value_average_.set_max_size(window_size);
+  this->average_.set_max_size(window_size);
 }
 optional<float> SlidingWindowMovingAverageFilter::new_value(float value) {
-  float average_value = this->value_average_.next_value(value);
+  float average_value = this->average_.next_value(value);
 
   if (++this->send_at_ >= this->send_every_) {
     this->send_at_ = 0;
@@ -44,11 +44,10 @@ uint32_t SlidingWindowMovingAverageFilter::expected_interval(uint32_t input) {
 
 ExponentialMovingAverageFilter::ExponentialMovingAverageFilter(float alpha, size_t send_every)
     : send_every_(send_every), send_at_(send_every - 1),
-      value_average_(ExponentialMovingAverage(alpha)),
-      accuracy_average_(ExponentialMovingAverage(alpha)) {
+      average_(ExponentialMovingAverage(alpha)) {
 }
 optional<float> ExponentialMovingAverageFilter::new_value(float value) {
-  float average_value = this->value_average_.next_value(value);
+  float average_value = this->average_.next_value(value);
 
   if (++this->send_at_ >= this->send_every_) {
     this->send_at_ = 0;
@@ -63,11 +62,10 @@ void ExponentialMovingAverageFilter::set_send_every(size_t send_every) {
   this->send_every_ = send_every;
 }
 float ExponentialMovingAverageFilter::get_alpha() const {
-  return this->value_average_.get_alpha();
+  return this->average_.get_alpha();
 }
 void ExponentialMovingAverageFilter::set_alpha(float alpha) {
-  this->value_average_.set_alpha(alpha);
-  this->accuracy_average_.set_alpha(alpha);
+  this->average_.set_alpha(alpha);
 }
 uint32_t ExponentialMovingAverageFilter::expected_interval(uint32_t input) {
   return input * this->send_every_;
@@ -122,14 +120,26 @@ uint32_t Filter::expected_interval(uint32_t input) {
 void Filter::input(float value) {
   optional<float> out = this->new_value(value);
   if (out.has_value())
-    this->output_(*out);
+    this->output(*out);
 }
-void Filter::initialize(std::function<void(float)> &&output) {
-  this->output_ = std::move(output);
+void Filter::output(float value) {
+  if (this->next_ == nullptr) {
+    this->parent_->send_state_to_frontend_internal_(value);
+  } else {
+    this->next_->new_value(value);
+  }
 }
-
-Filter::~Filter() {
-  delete this->next_;
+void Filter::initialize(Sensor *parent, Filter *next) {
+  this->parent_ = parent;
+  this->next_ = next;
+}
+uint32_t Filter::calculate_remaining_interval(uint32_t input) {
+  uint32_t this_interval = this->expected_interval(input);
+  if (this->next_ == nullptr) {
+    return this_interval;
+  } else {
+    return this->next_->calculate_remaining_interval(this_interval);
+  }
 }
 
 ThrottleFilter::ThrottleFilter(uint32_t min_time_between_inputs)
@@ -161,35 +171,34 @@ optional<float> DeltaFilter::new_value(float value) {
   return {};
 }
 OrFilter::OrFilter(std::vector<Filter *> filters)
-    : filters_(std::move(filters)) {
+    : filters_(std::move(filters)), phi_(this) {
 
 }
+OrFilter::PhiNode::PhiNode(OrFilter *parent) : parent_(parent) {}
+
+optional<float> OrFilter::PhiNode::new_value(float value) {
+  this->parent_->output(value);
+
+  return {};
+}
 optional<float> OrFilter::new_value(float value) {
   for (Filter *filter : this->filters_)
     filter->input(value);
 
   return {};
 }
-OrFilter::~OrFilter() {
-  delete this->next_;
-  for (Filter *filter : this->filters_) {
-    delete filter;
-  }
-}
-
-void OrFilter::initialize(std::function<void(float)> &&output) {
-  Filter::initialize(std::move(output));
+void OrFilter::initialize(Sensor *parent, Filter *next) {
+  Filter::initialize(parent, next);
   for (Filter *filter : this->filters_) {
-    filter->initialize([this](float value) {
-      this->output_(value);
-    });
+    filter->initialize(parent, &this->phi_);
   }
+  this->phi_.initialize(parent, nullptr);
 }
 
 uint32_t OrFilter::expected_interval(uint32_t input) {
   uint32_t min_interval = UINT32_MAX;
   for (Filter *filter : this->filters_) {
-    min_interval = std::min(min_interval, filter->expected_interval(input));
+    min_interval = std::min(min_interval, filter->calculate_remaining_interval(input));
   }
 
   return min_interval;
@@ -205,7 +214,7 @@ optional<float> UniqueFilter::new_value(float value) {
 
 optional<float> DebounceFilter::new_value(float value) {
   this->set_timeout("debounce", this->time_period_, [this, value](){
-    this->output_(value);
+    this->output(value);
   });
 
   return {};
@@ -226,6 +235,7 @@ HeartbeatFilter::HeartbeatFilter(uint32_t time_period)
 
 optional<float> HeartbeatFilter::new_value(float value) {
   this->last_input_ = value;
+  this->has_value_ = true;
 
   return {};
 }
@@ -234,7 +244,10 @@ uint32_t HeartbeatFilter::expected_interval(uint32_t input) {
 }
 void HeartbeatFilter::setup() {
   this->set_interval("heartbeat", this->time_period_, [this]() {
-    this->output_(this->last_input_);
+    if (!this->has_value_)
+      return;
+
+    this->output(this->last_input_);
   });
 }
 float HeartbeatFilter::get_setup_priority() const {

src/esphomelib/sensor/filter.h

@@ -36,21 +36,24 @@ class Filter {
    */
   virtual optional<float> new_value(float value) = 0;
 
-  virtual ~Filter();
-
-  virtual void initialize(std::function<void(float)> &&output);
+  /// Initialize this filter, please note this can be called more than once.
+  virtual void initialize(Sensor *parent, Filter *next);
 
   void input(float value);
 
   /// Return the amount of time that this filter is expected to take based on the input time interval.
   virtual uint32_t expected_interval(uint32_t input);
 
+  uint32_t calculate_remaining_interval(uint32_t input);
+
+  void output(float value);
+
  protected:
   friend Sensor;
   friend MQTTSensorComponent;
 
-  std::function<void(float)> output_;
   Filter *next_{nullptr};
+  Sensor *parent_{nullptr};
 };
 
 /** Simple sliding window moving average filter.
@@ -80,7 +83,7 @@ class SlidingWindowMovingAverageFilter : public Filter {
   uint32_t expected_interval(uint32_t input) override;
 
  protected:
-  SlidingWindowMovingAverage value_average_;
+  SlidingWindowMovingAverage average_;
   size_t send_every_;
   size_t send_at_;
 };
@@ -104,8 +107,7 @@ class ExponentialMovingAverageFilter : public Filter {
   uint32_t expected_interval(uint32_t input) override;
 
  protected:
-  ExponentialMovingAverage value_average_;
-  ExponentialMovingAverage accuracy_average_;
+  ExponentialMovingAverage average_;
   size_t send_every_;
   size_t send_at_;
 };
@@ -209,6 +211,7 @@ class HeartbeatFilter : public Filter, public Component {
  protected:
   uint32_t time_period_;
   float last_input_;
+  bool has_value_{false};
 };
 
 class DeltaFilter : public Filter {
@@ -226,14 +229,23 @@ class OrFilter : public Filter {
  public:
   explicit OrFilter(std::vector<Filter *> filters);
 
-  ~OrFilter() override;
-  void initialize(std::function<void(float)> &&output) override;
+  void initialize(Sensor *parent, Filter *next) override;
+
   uint32_t expected_interval(uint32_t input) override;
 
   optional<float> new_value(float value) override;
 
  protected:
+  class PhiNode : public Filter {
+   public:
+    PhiNode(OrFilter *parent);
+    optional<float> new_value(float value) override;
+   protected:
+    OrFilter *parent_;
+  };
+
   std::vector<Filter *> filters_;
+  PhiNode phi_;
 };
 
 class UniqueFilter : public Filter {

src/esphomelib/sensor/sensor.cpp

@@ -85,14 +85,9 @@ void Sensor::add_filter(Filter *filter) {
     Filter *last_filter = this->filter_list_;
     while (last_filter->next_ != nullptr)
       last_filter = last_filter->next_;
-    last_filter->next_ = filter;
-    last_filter->output_ = [filter] (float value) {
-      filter->input(value);
-    };
+    last_filter->initialize(this, filter);
   }
-  filter->initialize([this](float value) {
-    this->send_state_to_frontend_internal_(value);
-  });
+  filter->initialize(this, nullptr);
 }
 void Sensor::add_filters(const std::vector<Filter *> &filters) {
   for (Filter *filter : filters) {
@@ -104,13 +99,8 @@ void Sensor::set_filters(const std::vector<Filter *> &filters) {
   this->add_filters(filters);
 }
 void Sensor::clear_filters() {
-  Filter *filter = this->filter_list_;
-  while (filter != nullptr) {
-    Filter *next_filter = filter->next_;
-    delete filter;
-    filter = next_filter;
-  }
-
+  // note: not deallocating here, it makes the code faster (no virtual destructor)
+  // plus this is not called too often.
   this->filter_list_ = nullptr;
 }
 float Sensor::get_value() const {
@@ -152,12 +142,12 @@ uint32_t Sensor::calculate_expected_filter_update_interval() {
   if (interval == 4294967295UL)
     // update_interval: never
     return 0;
-  Filter *filter = this->filter_list_;
-  while (filter != nullptr) {
-    interval = filter->expected_interval(interval);
-    filter = filter->next_;
+
+  if (this->filter_list_ == nullptr) {
+    return interval;
   }
-  return interval;
+
+  return this->filter_list_->calculate_remaining_interval(interval);
 }
 
 PollingSensorComponent::PollingSensorComponent(const std::string &name, uint32_t update_interval)

src/esphomelib/sensor/sensor.h

@@ -155,6 +155,8 @@ class Sensor : public Nameable {
   /// Calculate the expected update interval for values that pass through all filters.
   uint32_t calculate_expected_filter_update_interval();
 
+  void send_state_to_frontend_internal_(float state);
+
  protected:
   /** Override this to set the Home Assistant unit of measurement for this sensor.
    *
@@ -175,8 +177,6 @@ class Sensor : public Nameable {
   /// Return the accuracy in decimals for this sensor.
   virtual int8_t accuracy_decimals();
 
-  void send_state_to_frontend_internal_(float state);
-
   CallbackManager<void(float)> raw_callback_; ///< Storage for raw state callbacks.
   CallbackManager<void(float)> callback_; ///< Storage for filtered state callbacks.
   optional<std::string> unit_of_measurement_; ///< Override the unit of measurement

src/esphomelib/wifi_component.cpp

@@ -1,12 +1,10 @@
 #include "esphomelib/wifi_component.h"
 
 #ifdef ARDUINO_ARCH_ESP32
-#include <WiFi.h>
-#include <esp_wifi.h>
+  #include <esp_wifi.h>
 #endif
 #ifdef ARDUINO_ARCH_ESP8266
-#include <ESP8266WiFi.h>
-#include <user_interface.h>
+  #include <user_interface.h>
 #endif
 
 #include <utility>

src/esphomelib/wifi_component.h

@@ -7,9 +7,11 @@
 #ifdef ARDUINO_ARCH_ESP32
   #include <esp_wifi.h>
   #include <WiFiType.h>
+  #include <WiFi.h>
 #endif
 #ifdef ARDUINO_ARCH_ESP8266
   #include <ESP8266WiFiType.h>
+  #include <ESP8266WiFi.h>
 #endif
 
 #include "esphomelib/component.h"