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
228 stars 121 forks source link

SinricPro.handle() possible issue #242

Closed josexxxxxx closed 2 years ago

josexxxxxx commented 2 years ago

SinricPro.handle seems to block my program. The device stops responding in certain periods of time, after a while it responds again. My aplication control lights that also can be activated manually with switches, the switches stop responding. This does not always happen - sometimes weeks go by without it happening. I'm using an arduino, the program it has no loops except the main one where SinricPro.handle () is. It seems that when there are problems communicating with the server SinricPro.handle takes too long and blocks the program. Before, I used Sinric (not pro) and this did not happen. It started to happen when I migrated to sinric pro. Please advice.

kakopappa commented 2 years ago

hi @josexxxxxx

If the issue is related to responses, it's likely a delay in the callback function eg: onPowerState is causing the issue. If you can upload the code without the secret keys we could help you to investigate.

josexxxxxx commented 2 years ago

Thank you very much for your prompt response. I appreciate your attention. I think what you suggest is correct, the bloking may be due to checking the switch. I attach the program

sivar2311 commented 2 years ago

HI @josexxxxxx

I am going through your sketch right now.

Can you explain what hardware components (switch, pushbutton) etc. do you use? This is not clear to me.

The code in mainloop should do the debouncing of 10ms, is that correct?

sivar2311 commented 2 years ago

Please, can you also explain your VerificaBoton function. Adding comments to every line would help me a lot.

I guess the variable names are in spanish language (which i do not speak)

josexxxxxx commented 2 years ago

cto cuartoPrinX.TXT

josexxxxxx commented 2 years ago

As you say "VerificaBoton()" is a debounce function for the switch I think the problem is in this debounce function, which waits for exact times of the main loop, if for some reason there is a delay, the switches are no longer detected. I think then that it is not a problem of SinricPro.handle. I hope to solve it by making a better design of the debounce function. Thanks for your time and advice, I close the issue

kakopappa commented 2 years ago

Silly question, why not use a library like OneButton to handle the button?

https://github.com/mathertel/OneButton

On Mon, 17 Jan 2022 at 7:01 AM josexxxxxx @.***> wrote:

As you say "VerificaBoton()" is a debounce function for the switch I think the problem is in this debounce function, which waits for exact times of the main loop, if for some reason there is a delay, the switches are no longer detected. I think then that it is not a problem of SinricPro.handle. I hope to solve it by making a better design of the debounce function. Thanks for your time and advice, I close the issue

— Reply to this email directly, view it on GitHub https://github.com/sinricpro/esp8266-esp32-sdk/issues/242#issuecomment-1013982999, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABZAZZTDB5D5KWUUC7U57W3UWNL5BANCNFSM5LZEYD2A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

sivar2311 commented 2 years ago

Hi @josexxxxxx

I rewrote your sketch. This one should work for you:

#include <Arduino.h>
#ifdef ESP8266
#include <ESP8266WiFi.h>
#endif
#ifdef ESP32
#include <WiFi.h>
#endif
#include <SinricPro.h>
#include <SinricProSwitch.h>

#define WIFI_SSID "<enter your wifi ssid here>"
#define WIFI_PASS "<enter your wifi pass here>"

#define APP_KEY    "<enter your sinricpro appkey here>"
#define APP_SECRET "<enter your sinricpro appsecret here>"

#define SWITCH_ID_1 "<enter your sinricpro device id here>"
#define SWITCH_ID_2 "<enter your sinricpro device id here>"

#define BUTTON_PIN_1 D7
#define BUTTON_PIN_2 D8 // warning D8 is pulled to ground! Boot fails if it is HIGH at boot!! (check https://randomnerdtutorials.com/esp8266-pinout-reference-gpios/)

#define RELAY_PIN_1 D5
#define RELAY_PIN_2 D6

#define DEBOUNCE_MILLIS 50

struct ButtonRelayConfig {
    String device_id;
    int    gpio_button;
    int    gpio_relay;

    // used for state change detection
    bool last_button_state;

    // used for debounce
    unsigned long last_millis;
};

// create an array of ButtonRelayConfig
ButtonRelayConfig buttons[] {
    //device_id, gpio_button, gpio_relay, last_button_state, last_millis
    {SWITCH_ID_1, BUTTON_PIN_1, RELAY_PIN_1, false, 0},
    {SWITCH_ID_2, BUTTON_PIN_2, RELAY_PIN_2, false, 0}
};

bool onPowerState(const String& deviceId, bool& state) {
    int i = 1;  // counter (used for printing the relay number)

    // for each button
    for (auto& button : buttons) {
        // check for a deviceId match
        if (deviceId == button.device_id) {
            Serial.printf("Relay %i turned %s\r\n", i, state ? "on" : "off");

            // turn gpio_relay on/off (whatever state currently is)
            digitalWrite(button.gpio_relay, state);

            // search is done
            return true;
        }
        i++;
    }

    // failed, no deviceId match!
    return false;
}

void handleButton(ButtonRelayConfig& button) {
    // debounce

    // get current millis
    unsigned long current_millis = millis();

    // if debounce time is not passed yet, return
    if (!button.last_millis && current_millis - button.last_millis < DEBOUNCE_MILLIS) return;

    // update debounce time
    button.last_millis = current_millis;

    // check button state
    bool current_state = digitalRead(button.gpio_button);

    // if state did not change, do nothing
    if (current_state == button.last_button_state) return;

    // if state did change, update button state
    button.last_button_state = current_state;

    // if button is pressed, flip relay state
    if (current_state == true) {
        digitalWrite(button.gpio_relay, !digitalRead(button.gpio_relay));
    }
}

void setupButtonRelays() {
    for (auto& button : buttons) {
        pinMode(button.gpio_button, INPUT);
        pinMode(button.gpio_relay, OUTPUT);
    }
}

void setupSinricPro() {
    // for each config
    for (auto& button : buttons) {
        // create a SinricProSwitch
        SinricProSwitch& relay = SinricPro[button.device_id];

        // assign onPowerState callback
        relay.onPowerState(onPowerState);
    };

    SinricPro.begin(APP_KEY, APP_SECRET);
}

void setup() {
    Serial.begin(115200);
    WiFi.begin(WIFI_SSID, WIFI_PASS);
    setupButtonRelays();
    setupSinricPro();
}

void loop() {
    SinricPro.handle();

    // for each button...handle that button
    for (auto& button : buttons) handleButton(button);
}
josexxxxxx commented 2 years ago

I didn't know, I'll try

josexxxxxx commented 2 years ago

thank you very much for the code!

sivar2311 commented 2 years ago

You are welcome. I hope the code is understandable for you.

If you have any questions I will be happy to answer them.