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
234 stars 124 forks source link

Doubt on restore device state function in sinric pro #231

Closed harry9713 closed 2 years ago

harry9713 commented 2 years ago

Hi, I am unable to restore the last device state with my ESP8266 which connected to a normal switch device and a Dimmer swtich device. I have given the true flag in SinricPro.restoreDeviceStates function. Still, the previous powerstate and powerlevel is not getting activated. Since I am new to coding I guess my implementation is incorrect somewhere. Hope someone can help. This is my complete code

#ifdef ENABLE_DEBUG
       #define DEBUG_ESP_PORT Serial
       #define NODEBUG_WEBSOCKETS
       #define NDEBUG
#endif 

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

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

#define WIFI_SSID         "xxxxx"    
#define WIFI_PASS         "xxxxx"
#define APP_KEY           "xxxxx"      // Should look like "de0bxxxx-1x3x-4x3x-ax2x-5dabxxxxxxxx"
#define APP_SECRET        "xxxxx"   // Should look like "5f36xxxx-x3x7-4x3x-xexe-e86724a9xxxx-4c4axxxx-3x3x-x5xe-x9x3-333d65xxxxxx"

#define SWITCH_ID_1       "xxxxx"    // Should look like "5dc1564130xxxxxxxxxxxxxx"
#define SWITCH_ID_2       "xxxxx"    // Should look like "5dc1564130xxxxxxxxxxxxxx"

#define BAUD_RATE         115200                // Change baudrate to your need
#define devicepwm1          12
#define devicelight1        13

struct {
  bool powerState = false;
  int powerLevel = 0;
} device1_state;           //for the dimmer switch

struct {
  bool powerState = false;
} device2_state;        //for the regular switch

bool onPowerState1(const String &deviceId, bool &state) {
  if (state){
    analogWrite(devicepwm1, 1023);}
  else{
    analogWrite(devicepwm1, 0);}
  device1_state.powerState = state;
  Serial.printf("Device 1 turned %s\r\n", state?"on":"off");
  return true; // request handled properly
}

bool onPowerLevel1(const String &deviceId, int &powerLevel) {
  device1_state.powerLevel = powerLevel;
  int analogValue = map(powerLevel, 0, 100, 0, 1023);  // maps values between 0..100 to 0..1023
  if (device1_state.powerState){
    analogWrite(devicepwm1, analogValue);
  }
  //Serial.printf("Device %s power level changed to %d\r\n", deviceId.c_str(), device1_state.powerLevel,"%");
  Serial.printf("Device 1 power level changed to %d\r\n", device1_state.powerLevel,"%");
  return true;
}

bool onPowerState2(const String &deviceId, bool &state) {
  device2_state.powerState = state;
  digitalWrite(devicelight1, state);
  Serial.printf("Device 2 turned %s\r\n", state?"on":"off");
  return true; // request handled properly
}

// setup function for WiFi connection
void setupWiFi() {
  Serial.printf("\r\n[Wifi]: Connecting");
  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

  SinricProDimSwitch& Dimmer = SinricPro[SWITCH_ID_1];
  Dimmer.onPowerState(onPowerState1);
  Dimmer.onPowerLevel(onPowerLevel1);

  SinricProSwitch& table_light = SinricPro[SWITCH_ID_2];
  table_light.onPowerState(onPowerState2);

  // 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);
  SinricPro.restoreDeviceStates(true);

}

// main setup function
void setup() {
  Serial.begin(BAUD_RATE); Serial.printf("\r\n\r\n");
  analogWriteRange(1023);
  pinMode(devicepwm1, OUTPUT);
  pinMode(devicelight1, OUTPUT);
  setupWiFi();
  setupSinricPro();
}

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

I have tried with,

  1. place the restoreDevice(true) before SinricPro.begin as seen in an older issue thread here.

I have also checked the activity log in SinricPro portal and saw this log entry about restore

Restoring the following devices [ undefined, undefined ] state from cloud.

sivar2311 commented 2 years ago

restoreDeviceStates must called before begin, otherwise this wont work. But the main issue seems to be the undefined states (the server doesnt have a state for your devices). You have to connect your device at least once and initialize the states by turning on/off and set the dimming value.

harry9713 commented 2 years ago

You have to connect your device at least once and initialize the states by turning on/off and set the dimming value.

I understood your statement and I have actually turned on both devices and set the dimmer to a certain power level. The devices did not restore this state after I unplugged the esp8266 inorder to mimic the power failure followed by plugging it back to power. And yes, i have tried this script with placing restore devices before begin.

sivar2311 commented 2 years ago

please enable serial logging, so you can see whats happening and what messages are sent and received.

i am on vacation this weekend, so my possibilities are very limited right (just for your info)

kakopappa commented 2 years ago

Restoring the following devices [ undefined, undefined ] state from cloud. <-- This is a bug in the server (last release) will be fixed ASAP

On Sat, Nov 27, 2021 at 4:30 AM Boris Jäger @.***> wrote:

please enable serial logging, so you can see whats happening and what messages are sent and received.

i am on vacation this weekend, so my possibilities are very limited right (just for your info)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sinricpro/esp8266-esp32-sdk/issues/231#issuecomment-980441983, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABZAZZXSABJRY5Q7J6CNU2DUN736RANCNFSM5I3KWGLA . 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.

harry9713 commented 2 years ago

<-- This is a bug in the server (last release) will be fixed ASAP

Thanks for the information. How can I know when it is fixed ?😀

harry9713 commented 2 years ago

please enable serial logging, so you can see whats happening and what messages are sent and received.

Sure. I will try it. You mean enabling the debug functionality in the code, right?

i am on vacation this weekend, so my possibilities are very limited right (just for your info)

Sorry to bother. I was having confusion. Thanks for the quick response, I'll wait untill the bug get fixed.

kakopappa commented 2 years ago

https://github.com/sinricpro/esp8266-esp32-sdk/blob/master/examples/Switch/Switch/Switch.ino#L18

Uncomment this line.

I have uploaded a fix already for the “undefined” error. Testing now

On Sat, 27 Nov 2021 at 10:49 AM Sreehari K @.***> wrote:

please enable serial logging, so you can see whats happening and what messages are sent and received.

Sure. I will try it. You mean enabling the debug functionality in the code, right?

i am on vacation this weekend, so my possibilities are very limited right (just for your info)

Sorry to bother. I was having confusion. Thanks for the quick response, I'll wait untill the bug get fixed.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/sinricpro/esp8266-esp32-sdk/issues/231#issuecomment-980496155, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABZAZZRYVGROJDMWU3A3HMDUOBIOFANCNFSM5I3KWGLA . 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.

harry9713 commented 2 years ago

I have uploaded a fix already for the “undefined” error. Testing now

Enabled the debug and it shows this in the debug output while connecting after a power failure (the devices were in the ON state during power failure).

restoreDeviceStates(true) before sinricPro.begin

restoredevicestates:false
ip:192.164.209.11
mac:2C:F2:32:71:3B:0D
platform:ESP8266
version:2.9.10
[SinricPro:Websocket]: receiving data
[SinricPro.handleReceiveQueue()]: 1 message(s) in receiveQueue
[SinricPro.handleReceiveQueue()]: Signature is valid. Processing message...
[SinricPro:extractTimestamp(): Got Timestamp 1637986235
kakopappa commented 2 years ago

restoredevicestates:false <<< should be true.

Did make the changes Boris mentioned?

On Sat, 27 Nov 2021 at 11:17 AM Sreehari K @.***> wrote:

I have uploaded a fix already for the “undefined” error. Testing now

Enabled the debug and it shows this in the debug output while connecting after a power failure(the devices were in the ON state during power failure)

restoredevicestates:false

ip:192.164.209.11

mac:2C:F2:32:71:3B:0D

platform:ESP8266

version:2.9.10

[SinricPro:Websocket]: receiving data

[SinricPro.handleReceiveQueue()]: 1 message(s) in receiveQueue

[SinricPro.handleReceiveQueue()]: Signature is valid. Processing message...

[SinricPro:extractTimestamp(): Got Timestamp 1637986235

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/sinricpro/esp8266-esp32-sdk/issues/231#issuecomment-980498939, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABZAZZUQTJOHL3K5QBJIK5DUOBLUTANCNFSM5I3KWGLA . 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.

harry9713 commented 2 years ago

restoredevicestates:false <<< should be true

It's already true as I mentioned in my comment. And yes i have adjusted the code to place it before sinricpro.begin .

harry9713 commented 2 years ago

So, now it works even if the activity log still shows

Restoring the following devices [ undefined, undefined ] state from cloud.

as before. Thank you for the quick response and fix @kakopappa .

However, a small thing I just noticed is that, the dimmer reboots with full brightness momentarily and then restores the powerlevel. I understand this behaviour as it restores the PowerState and then PowerLevel. And I have this piece of code for onPowerState function

bool onPowerState1(const String &deviceId, bool &state) {
  if (state){
    analogWrite(devicepwm1, 1023);}       //this turns on the switch with full powerlevel intially.
  else{
    analogWrite(devicepwm1, 0);}
  device1_state.powerState = state;
  Serial.printf("Device 1 turned %s\r\n", state?"on":"off");
  return true; // request handled properly
}

Yes, I want to turn on the swtich with full power whenever I turn it on, but should restore the previous power level directly after a powerfailure. I hope you got the idea. Anyway to implement this in the code ? Can I pass the powerlevel argument from the previous state ?

sivar2311 commented 2 years ago

You need to define global powerState and powerLevel variables and set their values and check them in the callback functions.

harry9713 commented 2 years ago

You need to define global powerState and powerLevel variables and set their values and check them in the callback functions.

I cannot understand this completely, thanks to my limited programming knowledge. But how to use global state and level variables in restore function? I thought may be if the restore function checks the powerlevel first, for dimswitch device and overrides the powerstate for that particular time, it should be ok.

Or should I use a light device instead? I don't know if the restore function behaves differently for different devices.

sivar2311 commented 2 years ago

When restoreDeviceStates is set to true, each callback function will be called directly after the esp is connected to the SinricPro server.

Btw. you have allready defined global variables (device1_state and device2_state).

In your onPowerState1 function, dont write 1023 directly. instead write the (mapped) value from device1_state.powerLevel (because thats the last known value).

harry9713 commented 2 years ago

When restoreDeviceStates is set to true, each callback function will be called directly after the esp is connected to the SinricPro server.

Btw. you have allready defined global variables (device1_state and device2_state).

In your onPowerState1 function, dont write 1023 directly. instead write the value from device1_state.powerLevel (because thats the last known value).

I have made the following changes in the onPowerState function which should be ok for the purpose I guess.

bool onPowerState1(const String &deviceId, bool &state) {
  int analogValue = map(device1_state.powerLevel, 0, 100, 0, 1023);
  if (state){
    analogWrite(devicepwm1, analogValue);
    }
  else{
    analogWrite(devicepwm1, 0);
    device1_state.powerLevel = 100;
    }
  device1_state.powerState = state;
  Serial.printf("Device 1 turned %s with power level %d\r\n", state?"on":"off", device1_state.powerLevel);
  return true; // request handled properly
}

In the serial debug output though, the device ID is not matching which causes creation of new device. This results the intial off state of both devices since they are new !? Strange. Because I just copy pasted the deviceID from sinricPro portal.

[SinricPro]: Device "619fece1c8056474f39b5a8e" does not exist. Creating new device
[SinricPro:add()]: Adding device with id "619fece1c8056474f39b5a8e".
[SinricPro]: Device "619f7c40c8056474f39b13f3" does not exist. Creating new device
[SinricPro:add()]: Adding device with id "619f7c40c8056474f39b13f3".

the activity log in SinricPro portal shows undefined devices as before.

Restoring the following devices [ undefined, undefined ] state from cloud.

sivar2311 commented 2 years ago
  1. remove "device1_state.powerLevel = 100;" from the powerState callback.

the value only changes in the powerLevel callback.

  1. Thats a misunderstanding. The debug message is correct. if a device is accessed by SinricPro[deviceId] for the first time, the sdk will create the device. (That is an SDK message, not a server sided message)
harry9713 commented 2 years ago
  1. remove "device1_state.powerLevel = 100;" from the powerState callback.

the value only changes in the powerLevel callback.

Sorry for that. I did it to turn on the device with 100% Powerlevel in the next turn on command. Now, it retains the powerlevel even after turning it off i.e, it gets activated with whatever powerlevel it was, before it turned off. I actually do not want that.

Good thing is that, now restore functionality works as expected. But I have to turn it on and make it 100% everytime or make it 100% before turning it off.

  1. Thats a misunderstanding. The debug message is correct. if a device is accessed by SinricPro[deviceId] for the first time, the sdk will create the device. (That is an SDK message, not a server sided message)

Oh I'm sorry, did not know that. Thanks for the correction 😅

sivar2311 commented 2 years ago

Can you describe the behavior you want to have (at first boot and in normal operating mode)

I am a bit confused now and do not get the reason why you are using restoreDeviceState.

harry9713 commented 2 years ago

Can you describe the behavior you want to have (at first boot and in normal operating mode)

I am a bit confused now and do not get the reason why you are using restoreDeviceState.

Sure. So at first boot i want it to be at 0% powerlevel like a regular switch off position. Then I can turn it on and make adjustments. This is the same when I turned it off myself i.e, even if it is at certain powerlevel at the time I turned it off, i want it to be at 100% when I turn on. Do not want to go to the previous powerlevel.

Power failure is the only time I want it to retain its previous state. Because I did not do that, so it has to be at whatever level it was at, before the power failure. That's why I used the restore state because all the variables will be reset after a power failure (I guess) so I can fetch the data about its previous state only from the portal and use it.

I hope this helps.😶

sivar2311 commented 2 years ago

Extend the state structures like this:

struct {
  bool powerState = false;
  int powerLevel = 100;
  first_time = true;
} device1_state;           //for the dimmer switch

struct {
  bool powerState = false;
  first_time = true;
} device2_state;        //for the regular switch

The inside the callbacks check the first_time variable. This gives you the ability to check if the callback is called for the first time and decide which value you want to apply (0, 100 or the last known value). At the end of the callback set first_time to false.

harry9713 commented 2 years ago

@sivar2311 Sorry for the delay. I was bit busy earlier.

The inside the callbacks check the first_time variable. This gives you the ability to check if the callback is called for the first time and decide which value you want to apply (0, 100 or the last known value). At the end of the callback set first_time to false.

I totally understand that logic and yes it works fine for checking the first boot. But I am afraid that is not the actual problem here. I want a way to check if,

I turned it off on purpose (on my command ofcourse!), or it turned off by the power failure.

I have tried to implement this by resetting the variable, when it turned off by command i.e, when state = false

struct {
  bool powerState = false;
  int powerLevel = 0;
  bool off_state = true;
} device1_state;

struct {
  bool powerState = false;
} device2_state;

bool onPowerState1(const String &deviceId, bool &state) {
  int analogValue = map(device1_state.powerLevel, 0, 100, 0, 1023);
  if (state){                            
    if (device1_state.off_state){               
      analogWrite(devicepwm1, 1023);
      device1_state.off_state = false;
      }
    else{
      analogWrite(devicepwm1, analogValue);
      }
    }
  else{
    analogWrite(devicepwm1, 0);
    device1_state.off_state= true;                  //resetting because the device turned off by me!
    }
  device1_state.powerState = state;
  Serial.printf("Device 1 turned %s with power level %d\r\n", state?"on":"off", device1_state.powerLevel);
  return true; // request handled properly
}

when I use this, on the first boot it works fine. But when the power failure happens, the states are not getting restored rather they are in off state.

On the serial debug output it shows this

appkey:46f4d4d2-cd2f-i35-a713-c20aff119b5b
deviceids:619fece1c8056474f39b5a8e;619f7c40c8056474f39b13f3
restoredevicestates:true ////////////////////////////////// here it is true
ip:192.168.29.11
mac:2C:F4:32:73:3B:0D
platform:ESP8266
version:2.9.10
[SinricPro:Websocket]: connected
Connected to SinricPro
[SinricPro:Websocket]: headers: 
appkey:46f4d4d2-cd2f-i5-a713-c20aff119b5b
deviceids:619fece1c8056439b5a8e;619f7c40c8056474f39b13f3
restoredevicestates:false  ///////////////////////////////////////// here it is false
ip:192.168.29.11
mac:2C:F4:32:73:3B:0D
platform:ESP8266
version:2.9.10
[SinricPro:Websocket]: receiving data
[SinricPro.handleReceiveQueue()]: 1 message(s) in receiveQueue
[SinricPro.handleReceiveQueue()]: Signature is valid. Processing message...
[SinricPro:extractTimestamp(): Got Timestamp 163819459

I don't know what is wrong, wheather it is the logic or the code behaves differently than I thought.

kakopappa commented 2 years ago

@harry9713 There's an issue in the server when a sudden connection reset happens it sends the restore settings to the previous WebSocket connection. Working on a fix for this.

harry9713 commented 2 years ago

@harry9713 There's an issue in the server when a sudden connection reset happens it sends the restore settings to the previous WebSocket connection. Working on a fix for this.

Sure. That's great. I hope it will be fixed real soon.

kakopappa commented 2 years ago

done

On Mon, Nov 29, 2021 at 5:21 PM Sreehari K @.***> wrote:

@harry9713 https://github.com/harry9713 There's an issue in the server when a sudden connection reset happens it sends the restore settings to the previous WebSocket connection. Working on a fix for this.

Sure. That's great. I hope it will be fixed real soon.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sinricpro/esp8266-esp32-sdk/issues/231#issuecomment-981496335, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABZAZZQP2XBFLMZBF3QCBBTUONHYZANCNFSM5I3KWGLA . 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.

sivar2311 commented 2 years ago

@kakopappa: Thanks! I have checked and verified - it is working :) @harry9713: I think this should work for you now:

bool onPowerState1(const String &deviceId, bool &state) {
  if (device1_state.off_state) {
      if (state) analogWrite(devicepwm1, map(device1_state.powerLevel, 0, 100, 0, 1023));
      device1_state.off_state = false;
  } else {
    digitalWrite(devicepwm1, state);
  }
  Serial.printf("Device 1 turned %s\r\n", state?"on":"off");
  device1_state.powerState = state;
  return true;
}

bool onPowerLevel1(const String &deviceId, int &powerLevel) {
  device1_state.powerLevel = powerLevel;
  if (device1_state.powerState){
    analogWrite(devicepwm1, map(powerLevel, 0, 100, 0, 1023));
  }
  Serial.printf("Device 1 power level changed to %d%%\r\n", device1_state.powerLevel);
  return true;
}
harry9713 commented 2 years ago

@sivar2311 @kakopappa GREAT !! It works like a charm. Thanks a lot guys. 🤩🤩

kakopappa commented 2 years ago

Hi @sivar2311

Like @harry9713 mentioned, in the SDK,

https://github.com/sinricpro/esp8266-esp32-sdk/blob/b17770e45c85499df0fa3fcdb0aaddafa6f9c8cc/src/SinricProWebsocket.h#L203

once connected, code changes restoredevicestates = false and set the headers which prints the false value. It looks like the SDK sent wrong values. Any idea what's the reason for this code?

sivar2311 commented 2 years ago

Yes, there is a reason for this: reconnect after loosing wifi / internet connection should not trigger restoreDeviceStates. restoreDeviceStates should only work on a power failure / ESP reset. After the first connect restoreDeviceStates is set to false. The logging is a bit ugly here and needs a bit fine tuning.

stale[bot] commented 2 years 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!

stale[bot] commented 2 years 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!