Revert "Add LED status to indicate wifi condition and packet handling (#229)" (#240)

This reverts commit a5364b488bdddbaaac67312522172082a4879beb.

README.md

@@ -70,7 +70,7 @@ You can find pre-compiled firmware images on the [releases](https://github.com/s
 
 This project uses [WiFiManager](https://github.com/tzapu/WiFiManager) to avoid the need to hardcode AP credentials in the firmware.
 
-When the ESP powers on, you should be able to see a network named "ESPXXXXX", with XXXXX being an identifier for your ESP. Connect to this AP and a window should pop up prompting you to enter WiFi credentials.  If your board has a built-in LED (or you wire up an LED), it will [flash to indicate the status](#led-status).
+When the ESP powers on, you should be able to see a network named "ESPXXXXX", with XXXXX being an identifier for your ESP. Connect to this AP and a window should pop up prompting you to enter WiFi credentials.
 
 The network password is "**milightHub**".
 
@@ -88,19 +88,6 @@ The HTTP endpoints (shown below) will be fully functional at this point. You sho
 
 ![Web UI](http://imgur.com/XNNigvL.png)
 
-## LED Status
-
-Some ESP boards have a built-in LED, on pin #2.  This LED will flash to indicate the current status of the hub:
-
-* Fast flash (on/off once per second) means the Wifi is not configured.  See [Configure Wifi](#configure-wifi) to configure the hub.
-* Occasional blips of light (a flicker of light every 1.5 seconds) means the hub is on wifi and ready to operate.
-* Rapid blips of light for brief periods (three rapid flashes) means packets are either detected from a device or are being sent to a device.
-* Solid light means the Wifi waited to be configured and gave up, or something went wrong with wifi configuration.
-
-You can configure the LED pin from the web console.  Note that pin means the GPIO number, not the D number ... for example, D2 is actually GPIO4 and therefore its pin 4.  If you specify the pin as a negative number, it will reverse the LED (the built-in LED on pin 2 is reversed, so the default is -2).
-
-If you want to wire up your own LED on a pin, such as on D2/GPIO4, put a wire from D2 to one side of a 220 ohm resister.  On the other side, connect it to the positive side (the longer wire) of a 3.3V LED.  Then connect the negative side of the LED (the shorter wire) to ground.  If you use a different voltage LED, or a high current LED, you will need to add a driver circuit.
-
 ## REST endpoints
 
 1. `GET /`. Opens web UI. 

lib/LEDStatus/LEDStatus.cpp

@@ -1,183 +0,0 @@
-#include "LEDStatus.h"
-
-// constructor defines which pin the LED is attached to
-LEDStatus::LEDStatus(int8_t ledPin) {
-  // if pin negative, reverse and set inverse on pin outputs
-  if (ledPin < 0) {
-    ledPin = -ledPin;
-    _inverse = true;
-  } else {
-    _inverse = false;
-  }
-  // set up the pin
-  _ledPin = ledPin;
-  pinMode(_ledPin, OUTPUT);
-  digitalWrite(_ledPin, _pinState(LOW));
-  _timer = millis();
-}
-
-// change pin at runtime
-void LEDStatus::changePin(int8_t ledPin) {
-  bool inverse;
-  // if pin negative, reverse and set inverse on pin outputs
-  if (ledPin < 0) {
-    ledPin = -ledPin;
-    inverse = true;
-  } else {
-    inverse = false;
-  }
-
-  if ((ledPin != _ledPin) && (inverse != _inverse)) {
-    // make sure old pin is off
-    digitalWrite(_ledPin, _pinState(LOW));
-    _ledPin = ledPin;
-    _inverse = inverse;
-    // and make sure new pin is also off
-    pinMode(_ledPin, OUTPUT);
-    digitalWrite(_ledPin, _pinState(LOW));
-  }
-}
-
-
-// identify how to flash the LED by mode, continuously until changed
-void LEDStatus::continuous(LEDStatus::LEDMode mode) {
-  uint16_t ledOffMs, ledOnMs;
-  _modeToTime(mode, ledOffMs, ledOnMs);
-  continuous(ledOffMs, ledOnMs);
-}
-
-// identify how to flash the LED by on/off times (in ms), continuously until changed
-void LEDStatus::continuous(uint16_t ledOffMs, uint16_t ledOnMs) {
-  _continuousOffMs = ledOffMs;
-  _continuousOnMs = ledOnMs;
-  _continuousCurrentlyOn = false;
-  // reset LED to off
-  if (_ledPin > 0) {
-    digitalWrite(_ledPin, _pinState(LOW));
-  }
-  // restart timer
-  _timer = millis();
-}
-
-// identify a one-shot LED action (overrides continuous until done) by mode
-void LEDStatus::oneshot(LEDStatus::LEDMode mode, uint8_t count) {
-  uint16_t ledOffMs, ledOnMs;
-  _modeToTime(mode, ledOffMs, ledOnMs);
-  oneshot(ledOffMs, ledOnMs, count);
-}
-
-// identify a one-shot LED action (overrides continuous until done) by times (in ms)
-void LEDStatus::oneshot(uint16_t ledOffMs, uint16_t ledOnMs, uint8_t count) {
-  _oneshotOffMs = ledOffMs;
-  _oneshotOnMs = ledOnMs;
-  _oneshotCountRemaining = count;
-  _oneshotCurrentlyOn = false;
-  // reset LED to off
-  if (_ledPin > 0) {
-    digitalWrite(_ledPin, _pinState(LOW));
-  }
-  // restart timer
-  _timer = millis();
-}
-
-// call this function in your loop - it will return quickly after calculating if any changes need to 
-// be made to the pin to flash the LED
-void LEDStatus::LEDStatus::handle() {
-  // is a pin defined?
-  if (_ledPin == 0) {
-    return;
-  }
-
-  // are we currently running a one-shot?
-  if (_oneshotCountRemaining > 0) {
-      if (_oneshotCurrentlyOn) {
-          if ((_timer + _oneshotOnMs) < millis()) {
-              if (_oneshotOffMs > 0) {
-                  digitalWrite(_ledPin, _pinState(LOW));
-              }
-              _oneshotCurrentlyOn = false;
-              --_oneshotCountRemaining;
-              if (_oneshotCountRemaining == 0) {
-                  _continuousCurrentlyOn = false;
-              }
-              _timer += _oneshotOnMs;
-          }
-      } else {
-          if ((_timer + _oneshotOffMs) < millis()) {
-            if (_oneshotOnMs > 0) {
-                digitalWrite(_ledPin, _pinState(HIGH));
-            }
-            _oneshotCurrentlyOn = true;
-            _timer += _oneshotOffMs;
-          }            
-      }
-  } else {
-    // operate using continuous
-    if (_continuousCurrentlyOn) {
-      if ((_timer + _continuousOnMs) < millis()) {
-        if (_continuousOffMs > 0) {
-          digitalWrite(_ledPin, _pinState(LOW));
-        }
-        _continuousCurrentlyOn = false;
-        _timer += _continuousOnMs;
-      }
-    } else {
-      if ((_timer + _continuousOffMs) < millis()) {
-        if (_continuousOnMs > 0) {
-          digitalWrite(_ledPin, _pinState(HIGH));
-        }
-        _continuousCurrentlyOn = true;
-        _timer += _continuousOffMs;
-      }
-    }
-  }
-}
-
-
-// private helper converts mode to on/off times in ms
-void LEDStatus::_modeToTime(LEDStatus::LEDMode mode, uint16_t& ledOffMs, uint16_t& ledOnMs) {
-  switch (mode) {
-    case LEDMode::Off:
-      ledOffMs = 1000;
-      ledOnMs = 0;
-      break;
-    case LEDMode::SlowToggle:
-      ledOffMs = 1000;
-      ledOnMs = 1000;
-      break;
-    case LEDMode::FastToggle:
-      ledOffMs = 100;
-      ledOnMs = 100;
-      break;
-    case LEDMode::SlowBlip:
-      ledOffMs = 1500;
-      ledOnMs = 50;
-      break;
-    case LEDMode::FastBlip:
-      ledOffMs = 333;
-      ledOnMs = 50;
-      break;
-    case LEDMode::On:
-      ledOffMs = 0;
-      ledOnMs = 1000;
-      break;
-    case LEDMode::Flicker:
-      ledOffMs = 50;
-      ledOnMs = 30;
-      break;
-    default:
-      Serial.printf_P(PSTR("LEDStatus::_modeToTime: Uknown LED mode %d\n"), mode);
-      ledOffMs = 500;
-      ledOnMs = 2000;
-      break;
-  }
-}
-
-// private helper to optionally inverse the LED
-uint8_t LEDStatus::_pinState(uint8_t val) {
-  if (_inverse) {
-    return (val == LOW) ? HIGH : LOW;
-  }
-  return val;
-}
-

lib/LEDStatus/LEDStatus.h

@@ -1,44 +0,0 @@
-#include <Arduino.h>
-
-#ifndef _LED_STATUS_H
-#define _LED_STATUS_H
-
-class LEDStatus {
-  public:
-    enum class LEDMode {
-      Off,
-      SlowToggle,
-      FastToggle,
-      SlowBlip,
-      FastBlip,
-      Flicker,
-      On
-    };
-    LEDStatus(int8_t ledPin);
-    void changePin(int8_t ledPin);
-    void continuous(LEDMode mode);
-    void continuous(uint16_t ledOffMs, uint16_t ledOnMs);
-    void oneshot(LEDMode mode, uint8_t count = 1);
-    void oneshot(uint16_t ledOffMs, uint16_t ledOnMs, uint8_t count = 1);
-
-    void handle();
-
-  private:
-    void _modeToTime(LEDMode mode, uint16_t& ledOffMs, uint16_t& ledOnMs);
-    uint8_t _pinState(uint8_t val);
-    uint8_t _ledPin;
-    bool _inverse;
-
-    uint16_t _continuousOffMs = 1000;
-    uint16_t _continuousOnMs = 0;
-    bool _continuousCurrentlyOn = false;
-
-    uint16_t _oneshotOffMs;
-    uint16_t _oneshotOnMs;
-    uint8_t _oneshotCountRemaining = 0;
-    bool _oneshotCurrentlyOn = false;
-
-    unsigned long _timer = 0;
-};
-
-#endif

lib/Settings/Settings.cpp

@@ -87,7 +87,6 @@ void Settings::patch(JsonObject& parsedSettings) {
     this->setIfPresent(parsedSettings, "ce_pin", cePin);
     this->setIfPresent(parsedSettings, "csn_pin", csnPin);
     this->setIfPresent(parsedSettings, "reset_pin", resetPin);
-    this->setIfPresent(parsedSettings, "led_pin", ledPin);
     this->setIfPresent(parsedSettings, "packet_repeats", packetRepeats);
     this->setIfPresent(parsedSettings, "http_repeat_factor", httpRepeatFactor);
     this->setIfPresent(parsedSettings, "auto_restart_period", _autoRestartPeriod);
@@ -163,7 +162,6 @@ void Settings::serialize(Stream& stream, const bool prettyPrint) {
   root["ce_pin"] = this->cePin;
   root["csn_pin"] = this->csnPin;
   root["reset_pin"] = this->resetPin;
-  root["led_pin"] = this->ledPin;
   root["radio_interface_type"] = typeToString(this->radioInterfaceType);
   root["packet_repeats"] = this->packetRepeats;
   root["http_repeat_factor"] = this->httpRepeatFactor;

lib/Settings/Settings.h

@@ -74,7 +74,6 @@ public:
     cePin(16),
     csnPin(15),
     resetPin(0),
-    ledPin(-2),
     radioInterfaceType(nRF24),
     deviceIds(NULL),
     gatewayConfigs(NULL),
@@ -131,7 +130,6 @@ public:
   uint8_t cePin;
   uint8_t csnPin;
   uint8_t resetPin;
-  int8_t ledPin;
   RadioInterfaceType radioInterfaceType;
   uint16_t *deviceIds;
   GatewayConfig **gatewayConfigs;

platformio.ini

@@ -16,8 +16,7 @@ lib_deps_builtin =
   SPI
 lib_deps_external =
   sidoh/RF24
-;  WiFiManager
-  https://github.com/cmidgley/WiFiManager
+  WiFiManager
   ArduinoJson
   PubSubClient
   https://github.com/ratkins/RGBConverter

src/main.cpp

@@ -19,12 +19,9 @@
 #include <MiLightDiscoveryServer.h>
 #include <MiLightClient.h>
 #include <BulbStateUpdater.h>
-#include <LEDStatus.h>
 
 WiFiManager wifiManager;
 
-static LEDStatus *ledStatus;
-
 Settings settings;
 
 MiLightClient* milightClient = NULL;
@@ -89,10 +86,6 @@ void onPacketSentHandler(uint8_t* packet, const MiLightRemoteConfig& config) {
   JsonObject& result = buffer.createObject();
   BulbId bulbId = config.packetFormatter->parsePacket(packet, result, stateStore);
 
-
-  // blip LED to indicate we saw a packet (send or receive)
-  ledStatus->oneshot(LEDStatus::LEDMode::Flicker, 3);
-
   if (&bulbId == &DEFAULT_BULB_ID) {
     Serial.println(F("Skipping packet handler because packet was not decoded"));
     return;
@@ -101,7 +94,6 @@ void onPacketSentHandler(uint8_t* packet, const MiLightRemoteConfig& config) {
   const MiLightRemoteConfig& remoteConfig =
     *MiLightRemoteConfig::fromType(bulbId.deviceType);
 
-  // update state to reflect changes from this packet
   GroupState& groupState = stateStore->get(bulbId);
   groupState.patch(result);
   stateStore->set(bulbId, groupState);
@@ -149,7 +141,6 @@ void handleListen() {
         return;
       }
 
-      // update state to reflect this packet
       onPacketSentHandler(readPacket, *remoteConfig);
     }
   }
@@ -233,10 +224,6 @@ void applySettings() {
     discoveryServer = new MiLightDiscoveryServer(settings);
     discoveryServer->begin();
   }
-
-  // update LED pin
-  if (ledStatus)
-    ledStatus->changePin(settings.ledPin);
 }
 
 /**
@@ -250,43 +237,20 @@ bool shouldRestart() {
   return settings.getAutoRestartPeriod()*60*1000 < millis();
 }
 
-// give a bit of time to update the status LED
-void handleLED() {
-  ledStatus->handle();
-}
-
 void setup() {
   Serial.begin(9600);
   String ssid = "ESP" + String(ESP.getChipId());
-  
-  // load up our persistent settings from the file system
+
+  wifiManager.setConfigPortalTimeout(180);
+  wifiManager.autoConnect(ssid.c_str(), "milightHub");
+
   SPIFFS.begin();
   Settings::load(settings);
   applySettings();
 
-  // set up the LED status
-  ledStatus = new LEDStatus(settings.ledPin);
-  ledStatus->continuous(LEDStatus::LEDMode::FastToggle);
-
-  // start up the wifi manager
   if (! MDNS.begin("milight-hub")) {
     Serial.println(F("Error setting up MDNS responder"));
   }
-  // tell Wifi manager to call us during the setup.  Note that this "setSetupLoopCallback" is an addition
-  // made to Wifi manager in a private fork.  As of this writing, WifiManager has a new feature coming that
-  // allows the "autoConnect" method to be non-blocking which can implement this same functionality.  However,
-  // that change is only on the development branch so we are going to continue to use this fork until
-  // that is merged and ready.
-  wifiManager.setSetupLoopCallback(handleLED);
-  wifiManager.setConfigPortalTimeout(180);
-  if (wifiManager.autoConnect(ssid.c_str(), "milightHub")) {
-    ledStatus->continuous(LEDStatus::LEDMode::SlowBlip);
-    Serial.println(F("Wifi connected succesfully\n"));
-  } else {
-    ledStatus->continuous(LEDStatus::LEDMode::On);
-    Serial.println(F("Wifi failed.  Oh well.\n"));
-  }
-
 
   MDNS.addService("http", "tcp", 80);
 
@@ -328,9 +292,6 @@ void loop() {
 
   stateStore->limitedFlush();
 
-  // update LED with status
-  ledStatus->handle();
-
   if (shouldRestart()) {
     Serial.println(F("Auto-restart triggered. Restarting..."));
     ESP.restart();

web/src/js/script.js

@@ -5,7 +5,7 @@ var UNIT_PARAMS = {
 };
 
 var FORM_SETTINGS = [
-  "admin_username", "admin_password", "ce_pin", "csn_pin", "reset_pin","led_pin", "packet_repeats",
+  "admin_username", "admin_password", "ce_pin", "csn_pin", "reset_pin","packet_repeats",
   "http_repeat_factor", "auto_restart_period", "discovery_port", "mqtt_server",
   "mqtt_topic_pattern", "mqtt_update_topic_pattern", "mqtt_state_topic_pattern",
   "mqtt_username", "mqtt_password", "radio_interface_type", "listen_repeats",
@@ -32,7 +32,6 @@ var GROUP_STATE_KEYS = [
 
 var FORM_SETTINGS_HELP = {
   ce_pin : "'CE' for NRF24L01 interface, and 'PKT' for 'PL1167/LT8900' interface",
-  led_pin : "Pin to use for LED status display (0=disabled); negative inverses signal (recommend -2 for on-board LED)",
   packet_repeats : "The number of times to repeat RF packets sent to bulbs",
   http_repeat_factor : "Multiplicative factor on packet_repeats for " +
     "requests initiated by the HTTP API. UDP API typically receives " +