sinricpro / esp8266-esp32-sdk

Library for https://sinric.pro - simple way to connect your device to Alexa, Google Home, SmartThings and cloud
https://sinric.pro
Other
236 stars 125 forks source link

Relay status not syncing correctly with Sinric Pro dashboard #364

Closed czaremanuel closed 9 months ago

czaremanuel commented 9 months ago

Hello all,

I am trying to create a simple on/off relay using an ESP-01S and Arduino IDE. My goal is that when I press "Turn on" from the Sinric dash, the relay is activated for 1 second, then turns off. So from a hardware issue, that process works. However it is not accurately reflecting the device state within the Sinric dash.

What happens right now is that the device does indeed power on for one second and turn off, so the hardware is functioning correctly, however the Sinric Dash reports the device is off for a brief moment, and then changes the status to incorrectly display the device is "on." It would be helpful to have the device revert to "off" in Sinric after that one second elapses. I am not sure what I got wrong in my code (below). Any help would be much appreciated. Thank you!

#include <Arduino.h>
#include <ESP8266WiFi.h>

#include "SinricPro.h"
#include "SinricProSwitch.h"

#define WIFI_SSID         "---"
#define WIFI_PASS         "---"
#define APP_KEY           "---"
#define APP_SECRET        "---"

#define SWITCH_ID_1       "---"
#define RELAYPIN_1        2

#define BAUD_RATE         115200

bool myPowerState = false; // track device state

bool onPowerState1(const String &deviceId, bool &state) {
  Serial.printf("Device 1 turned %s\r\n", state ? "on" : "off");

  // Update local power state immediately
  myPowerState = state;

  // Control the relay based on the new state
  digitalWrite(RELAYPIN_1, state ? LOW : HIGH);

  // If device was turned on, handle automatic turn-off after 1 second
  if (state) {
    unsigned long startTime = millis();

    while (millis() - startTime < 1000) {
      // Do any other actions needed while on (optional)
      // ...
      delay(10); // Small delay for responsiveness
    }

    Serial.println("Turning device off...");
    digitalWrite(RELAYPIN_1, HIGH); // Turn off the relay

    // Report the updated state to Sinric
    SinricProSwitch &mySwitch = SinricPro[SWITCH_ID_1];
    mySwitch.sendPowerStateEvent(false); // Reflect the "off" state
  }

  return true; // request handled properly
}

// setup function for WiFi connection
void setupWiFi() {
  Serial.printf("\r\n[Wifi]: Connecting");

  #if defined(ESP8266)
    WiFi.setSleepMode(WIFI_NONE_SLEEP); 
    WiFi.setAutoReconnect(true);
  #elif defined(ESP32)
    WiFi.setSleep(false); 
    WiFi.setAutoReconnect(true);
  #endif

  WiFi.begin(WIFI_SSID, WIFI_PASS);

  while (WiFi.status() != WL_CONNECTED) {
    Serial.printf(".");
    delay(250);
  }

  Serial.printf("connected!\r\n[WiFi]: IP-Address is %s\r\n", WiFi.localIP().toString().c_str());
}

// setup function for SinricPro
void setupSinricPro() {
  // add devices and callbacks to SinricPro
  pinMode(RELAYPIN_1, OUTPUT);
  digitalWrite(RELAYPIN_1, HIGH);

  SinricProSwitch& mySwitch1 = SinricPro[SWITCH_ID_1];
  mySwitch1.onPowerState(onPowerState1);

  // setup SinricPro
  SinricPro.onConnected([](){ Serial.printf("Connected to SinricPro\r\n"); }); 
  SinricPro.onDisconnected([](){ Serial.printf("Disconnected from SinricPro\r\n"); });

  SinricPro.begin(APP_KEY, APP_SECRET);
}

// main setup function
void setup() {
  Serial.begin(BAUD_RATE); Serial.printf("\r\n\r\n");
  setupWiFi();
  setupSinricPro();
}

void loop() {
  SinricPro.handle();
}
sivar2311 commented 9 months ago

One second is far too short for meaningful feedback to the server. In this case, it is advisable to report the status "Off" directly. Insert this line at the end of the onPowerState1 function (before return true):

  state = false;

Note: You are using a blocking loop in the callback function. You should avoid this at all costs!

It is better to set a global flag in the callback function which is then evaluated and processed in the main loop.

czaremanuel commented 9 months ago

One second is far too short for meaningful feedback to the server. In this case, it is advisable to report the status "Off" directly.

Well I'm happy to say this worked, thank you.

Note: You are using a blocking loop in the callback function. You should avoid this at all costs! It is better to set a global flag in the callback function which is then evaluated and processed in the main loop.

Would you mind explaining what this means in greater detail? I am still learning all these concepts.

sivar2311 commented 9 months ago

I have written a "clean-code" example for you. It contains a few more lines, but it should be easy to read and you should be able to understand how it works.

#include <Arduino.h>
#include <ESP8266WiFi.h>

#include "SinricPro.h"
#include "SinricProSwitch.h"

#define WIFI_SSID  "---"
#define WIFI_PASS  "---"
#define APP_KEY    "---"
#define APP_SECRET "---"

#define SWITCH_ID_1 "---"
#define RELAYPIN_1  2

#define BAUD_RATE 115200

// here you can choose the duration for the "on"-time
const uint32_t timerDuration = 1000;

// used as timer
uint32_t timerStart = 0;

// some "clean-code" low-level helper functions to make the "high-level-code" easy to read

void turnOnRelay() {
    digitalWrite(RELAYPIN_1, LOW);
}

void turnOffRelay() {
    digitalWrite(RELAYPIN_1, HIGH);
}

void startTimer() {
    timerStart = millis();
}

void stopTimer() {
    timerStart = 0;
}

bool timerIsRunning() {
    return timerStart != 0;
}

bool durationIsOver() {
    return millis() - timerStart >= timerDuration;
}

// some "clean-code" mid-level-functions

void turnOnRelayAndStartTheTimer() {
    turnOnRelay();
    startTimer();
}

void turnOffRelayAndStopTheTimer() {
    turnOffRelay();
    stopTimer();
}

// high-level function used in loop() later...

void handleRelayTimer() {
    if (timerIsRunning() && durationIsOver()) turnOffRelayAndStopTheTimer();  // code speaks for itself
}

// onPowerState handler (high-level code)

bool onPowerState1(const String &deviceId, bool &state) {
    Serial.printf("Device 1 turned %s\r\n", state ? "on" : "off");

    // again a little "clean code" helper to make the following line easy to read
    bool isTurnOnRequest = (state == true);

    if (isTurnOnRequest) turnOnRelayAndStartTheTimer();  // code speaks for itself

    state = false;  // respond to server "relay is off"
    return true;    // request handled properly
}

// setup function for WiFi connection
void setupWiFi() {
    Serial.printf("\r\n[Wifi]: Connecting");

    WiFi.setSleepMode(WIFI_NONE_SLEEP);
    WiFi.setAutoReconnect(true);
    WiFi.begin(WIFI_SSID, WIFI_PASS);

    while (WiFi.status() != WL_CONNECTED) {
        Serial.printf(".");
        delay(250);
    }

    Serial.printf("connected!\r\n[WiFi]: IP-Address is %s\r\n", WiFi.localIP().toString().c_str());
}

// setup function for SinricPro
void setupSinricPro() {
    // add devices and callbacks to SinricPro
    SinricProSwitch &mySwitch1 = SinricPro[SWITCH_ID_1];
    mySwitch1.onPowerState(onPowerState1);

    // setup SinricPro
    SinricPro.onConnected([]() { Serial.printf("Connected to SinricPro\r\n"); });
    SinricPro.onDisconnected([]() { Serial.printf("Disconnected from SinricPro\r\n"); });

    SinricPro.begin(APP_KEY, APP_SECRET);
}

void setupRelay() {
    pinMode(RELAYPIN_1, OUTPUT);
    turnOffRelay();
}

// main setup function
void setup() {
    Serial.begin(BAUD_RATE);
    Serial.printf("\r\n\r\n");
    setupRelay();
    setupWiFi();
    setupSinricPro();
}

void loop() {
    SinricPro.handle();
    handleRelayTimer();
}

Note: The code compiles, but has not been tested for functionality.

github-actions[bot] commented 9 months ago

This issue has gone quiet. Spooky quiet. We currently close issues after 14 days of inactivity. It’s been at least 7 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. As a friendly reminder, the best way to fix this or any other problem is to provide a detailed error description including a serial log. Thanks for being a part of the SinricPro community!

github-actions[bot] commented 9 months ago

Hey again! It’s been 14 days since anything happened on this issue, so our friendly robot (that’s me!) is going to close it. Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to comment on this issue or create a new one if you need anything else. As a friendly reminder, the best way to fix this or any other problem is to provide a detailed error description including a serial log. Thanks again for being a part of the SinricPro community!