Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

timer offset setting '-00:00' causes 12:00 hour offset #3923

Closed
gitMichael opened this issue Sep 28, 2018 · 25 comments
Closed

timer offset setting '-00:00' causes 12:00 hour offset #3923

gitMichael opened this issue Sep 28, 2018 · 25 comments
Labels
awaiting feedback Action - Waiting for response or more information bug Type - Confirmated Bug fixed Result - The work on the issue has ended

Comments

@gitMichael
Copy link

You can set timer offset to '-00:00' or '00:00' on webpage.
But one thing creates a 12:00 hours offset, the other the right 00:00 hours.

Reason: Pure sign interpretation in 'xdrv_09_timers.ino' line 395, I guess quickly looked...
@ascillato2 ascillato2 added the bug Type - Confirmated Bug label Sep 28, 2018
@arendst
Copy link
Owner

arendst commented Sep 29, 2018 •
edited
Loading

Pls provide the information requested when you started this issue.

At least the output of command status 0.

@arendst arendst added question Type - Asking for Information and removed bug Type - Confirmated Bug labels Sep 29, 2018
@arendst
Copy link
Owner

arendst commented Sep 29, 2018

Pls also show a situation where it fails.

Background: a minus offset is always incremented by 720 minutes (12 hours). This is used to calculate the correct trigger time using the sunrise/sunset start times.

@gitMichael
Copy link
Author

Sorry but I am afraid I am not of the patient kind, rather choleric in technical things.
So don't worry, I only want to help! :-)
True to my pleasant motto: Pls don't wast my time with your bugs.

Read my post. I wrote timer offset, that means offset (I tried out from sunset accidentally).
Pls take a look at your code.

a minus offset is always incremented by 720 minutes (12 hours). This is used to calculate the correct trigger time using the sunrise/sunset start times.

Yes, but '-00:00' sets offset to 720 and '00:00' to 0. And in your code is 720 the max. positive time. Don't you really see the problem? It's a simple input range error.

Do not be afraid, it was a hard day and it's late and just fun!

@meingraham
Copy link
Collaborator

@gitMichael: "Pls don't wast my time with your bugs." Yikes! How to make friends and influence people ;-)

@Jason2866
Copy link
Collaborator

@gitMichael
It seems it so easy for you to correct. Just do a PR 😀

@andrethomas
Copy link
Contributor

I agree with @arendst and I don't think there is a way to create a real use case scenario for this to be considered a bug.

@gitMichael
Copy link
Author

Be careful to say 'do not' in complex systems.
grafik

@arendst
Copy link
Owner

arendst commented Sep 29, 2018

And it still doesn't fail as far as I can see.

Line 395 is not used by the webinterface.

@andrethomas
Copy link
Contributor

@gitMichael Line 395 isn't relevant to the bug you are experiencing. Perhaps if you want to debug read the code, make changes, and see if you can confirm it? ;) Unless of course, you're too busy to do that.

@ascillato
Copy link
Contributor

Hi,

I can confirm that if you set a timer for sunset and chose - 00:00 as offset it will trigger at sunset minus 12 hours.

@arendst arendst added bug Type - Confirmated Bug and removed question Type - Asking for Information labels Sep 30, 2018
@andrethomas
Copy link
Contributor

andrethomas commented Sep 30, 2018 •
edited
Loading

Using sunrise +00:00

15:29:37 HTP: Configure Timer
15:29:42 HTP: Save configuration
-1199603712,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
String part is '-1199603712,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0,0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0'
String part is '0,0,0,0,0'
String part is '0,0,0,0'
String part is '0,0,0'
String part is '0,0'
String part is '0'
15:29:42 MQT: Timers 1,0xB87F8000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000

Using sunrise -00:00

15:31:11 HTP: Configure Timer
15:31:16 HTP: Save configuration
-1199602992,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
String part is '-1199602992,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0,0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0,0'
String part is '0,0,0,0,0,0'
String part is '0,0,0,0,0'
String part is '0,0,0,0'
String part is '0,0,0'
String part is '0,0'
String part is '0'
15:31:16 MQT: Timers 1,0xB87F82D0,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000

Settings.timer[i].data = 0xB87F8000 for + 00:00
and
Settings.timer[i].data = 0xB87F82D0 for - 00:00

Math says the difference of 0x2D0, or 720 and 720/60 = 12 hours

This difference is already present in the t0 parameter passed by save settings so it must be on the javacript side and not in the handling code.

To better understand how I got the output, see the Serial.print()'s I added

void TimerSaveSettings()
{
  char tmp[MAX_TIMERS *12];  // Need space for MAX_TIMERS x 10 digit numbers separated by a comma
  Timer timer;

  Settings.flag3.timers_enable = WebServer->hasArg("e0");
  WebGetArg("t0", tmp, sizeof(tmp));
  char *p = tmp;
  snprintf_P(log_data, sizeof(log_data), PSTR(D_LOG_MQTT D_CMND_TIMERS " %d"), Settings.flag3.timers_enable);
  Serial.println(p);
  
  for (byte i = 0; i < MAX_TIMERS; i++) {
    Serial.print("String part is '");
    Serial.print(p);
    Serial.println("'");
    
    timer.data = strtol(p, &p, 10);
    p++;  // Skip comma
    if (timer.time < 1440) {
      bool flag = (timer.window != Settings.timer[i].window);
      Settings.timer[i].data = timer.data;
      if (flag) TimerSetRandomWindow(i);
    }
    snprintf_P(log_data, sizeof(log_data), PSTR("%s,0x%08X"), log_data, Settings.timer[i].data);
  }
  AddLog(LOG_LEVEL_DEBUG);
}

@arendst
Copy link
Owner

arendst commented Sep 30, 2018

Thx for testing.

I'll dive in to the java part.

Can anyone verify that it works fine using the command line interface?

@arendst
Copy link
Owner

arendst commented Sep 30, 2018

Just did a test with an offset of -01:56 from sunset (=19:12) expected at 17:16 and that worked correctly:

17:13:49 HTP: Configure Timer
17:14:31 HTP: Save configuration
17:14:31 MQT: Timers 1,0xD07F0344,0x18040406,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000
17:14:31 HTP: Configuration
17:14:31 CFG: Saved to flash at F4, Count 386, Bytes 3584
timer1
17:14:43 CMD: timer1
17:14:43 SRC: Serial
17:14:43 RSL: Group 0, Index 1, Command TIMER, Data 
17:14:43 MQT: stat/ring2/TIMER = {"Timer1":{"Arm":1,"Mode":2,"Time":"-01:56","Window":0,"Days":"1111111","Repeat":0,"Output":1,"Action":2}}
17:16:00 SRC: Timer
17:16:00 MQT: stat/ring2/POWER = {"POWER":"ON"}
17:16:00 MQT: stat/ring2/POWER = ON
17:16:01 CFG: Saved to flash at FB, Count 387, Bytes 3584

@ascillato
Copy link
Contributor

ascillato commented Sep 30, 2018 via email

@arendst
Copy link
Owner

arendst commented Sep 30, 2018

Right.

So the solution is just to change the range check on line 197 from

  if ((uint16_t)stored.time > 720) {

to

  if ((uint16_t)stored.time > 719) {

arendst added a commit that referenced this issue Sep 30, 2018
@arendst
Fix timer offset -00:00
b9b6d13 
Fix timer offset -00:00 causing 12:00 hour offset (#3923)
@arendst arendst added fixed Result - The work on the issue has ended awaiting feedback Action - Waiting for response or more information labels Sep 30, 2018
@gitMichael
Copy link
Author

With that may be you get a 12:00 offset problem, I think.
It's just an input range problem. -0 is not a valid integer value.
It's nesseary to eliminate the '-' for an input time of '00:00' in all input functions e.g. at line 395 and in the webserver... aso imo ;-)

@andrethomas
Copy link
Contributor

@gitMichael

-0 is not a valid integer value.

Neither is 00:00 or 00:02 or -00:05 and I don't see anyone disputing this.

In embedded development one tends to use what is given to you to get the certain behaviour to work the way you want it. Time is not an exact science and embedded development certainly makes no exception to that.

I can't help but wonder... 2 day old github account with a username starting with git ? Does the prefix mean anything to you? I think if you apply the same logic to the way a negative is used here it might just, by a small fractional possibility, make sense.

Lastly, if you took some time to dig into the code you will note that line 395 in the web server code is not even executed when using a positive or negative time offset during configuration.

Thanks for your bug report though ;)

@andrethomas
Copy link
Contributor

@gitMichael Tested your theory on a possible 12 hour offset.

Sunrise for me tomorrow morning is 07:48, so I offset -11:45 which would cause a trigger at 20h03 for me in this case.

20:02:05 MQT: stat/sonoff/RESULT = {"Timer1":{"Arm":1,"Mode":1,"Time":"-11:45","Window":0,"Days":"1111111","Repeat":1,"Output":1,"Action":1}}
20:03:00 SRC: Timer
20:03:00 MQT: stat/sonoff/RESULT = {"POWER":"ON"}
20:03:00 MQT: stat/sonoff/POWER = ON
20:03:00 CFG: Saved to flash at 3F6, Count 28, Bytes 3584

Worked as expected!

@andrethomas
Copy link
Contributor

Ok, final test for -00:00

I configured a separate NTP server to be at a time just before sunrise, 07:40 for sunrise 07:48

image

Output on serial from config:

07:42:22 HTP: Save configuration
07:42:22 MQT: Timers 1,0xB07F82D0,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000
07:42:22 HTP: Configuration
07:42:23 CFG: Saved to flash at 3F4, Count 8, Bytes 3584

And as expected, the timer ran at sunrise.

07:48:22 SRC: Timer
07:48:22 MQT: stat/sonoff/RESULT = {"POWER":"ON"}
07:48:22 MQT: stat/sonoff/POWER = ON
07:48:22 CFG: Saved to flash at 3F5, Count 15, Bytes 3584
07:48:22 HTP: Configuration
07:51:39 HTP: Configure Timer
07:52:37 MQT: tele/sonoff/STATE = {"Time":"2018-09-30T07:52:37","Uptime":"0T00:05:16","V

@arendst @ascillato2 I think we can close this one up.

@ascillato
Copy link
Contributor

ascillato commented Sep 30, 2018 via email

@andrethomas
Copy link
Contributor

For posterity, I reset the NTP server back to 07h47 and performed a reboot on the sonoff - Observe output

00:00:00 APP: Boot Count 6
00:00:00 SRC: Restart
00:00:00 Project sonoff Sonoff (Topic sonoff, Fallback DVES_10373D, GroupTopic sonoffs) Version 6.2.1.10-2_3_0
00:00:00 CFG: Saved to flash at 3F4, Count 16, Bytes 3584
00:00:00 WIF: Attempting connection...
00:00:00 WIF: Patch issue 2186
00:00:00 WIF: Connecting to AP1 Wireless in mode 11N as sonoff-5949...
00:00:01 WIF: Attempting connection...
00:00:02 WIF: Attempting connection...
00:00:03 WIF: Attempting connection...
00:00:04 WIF: Attempting connection...
00:00:05 WIF: Attempting connection...
00:00:06 WIF: Connected
00:00:06 DNS: Initialized
00:00:06 HTP: Web server active on sonoff-5949.local with IP address 192.168.42.123
00:00:07 MQT: Attempting connection...
00:00:07 MQT: Connected
00:00:07 MQT: tele/sonoff/LWT = Online (retained)
00:00:07 MQT: cmnd/sonoff/POWER = 
00:00:07 MQT: Subscribe to cmnd/sonoff/#
00:00:07 MQT: Subscribe to cmnd/sonoffs/#
00:00:07 MQT: Subscribe to cmnd/DVES_10373D/#
00:00:07 MQT: tele/sonoff/INFO1 = {"Module":"Generic","Version":"6.2.1.10","FallbackTopic":"DVES_10373D","GroupTopic":"sonoffs"}
00:00:07 MQT: tele/sonoff/INFO2 = {"WebServerMode":"Admin","Hostname":"sonoff-5949","IPAddress":"192.168.42.123"}
00:00:07 MQT: tele/sonoff/INFO3 = {"RestartReason":"Software/System restart"}
00:00:07 MQT: stat/sonoff/RESULT = {"POWER":"ON"}
00:00:07 MQT: stat/sonoff/POWER = ON
00:00:08 APP: (UTC) Sun Sep 30 05:47:21 2018, (DST) Sun Mar 25 02:00:00 2018, (STD) Sun Oct 28 03:00:00 2018
07:47:27 CMD: timers
07:47:27 SRC: Serial
07:47:27 RSL: Group 0, Index 1, Command TIMERS, Data 
07:47:27 MQT: stat/sonoff/RESULT = {"Timers":"ON"}
07:47:27 MQT: stat/sonoff/RESULT = {"Timers1":{"Timer1":{"Arm":1,"Mode":1,"Time":"-00:00","Window":0,"Days":"1111111","Repeat":1,"Output":1,"Action":2},"Timer2":{"Arm":0,"Mode":0,"Time":"00:00","Window":0,"Days":"0000000","Repeat":0,"Output":1,"Action":0},"Timer3":{"Arm":0,"Mode":0,"Time":"00:00","Window":0,"Days":"0000000","Repeat":0,"Output":1,"Action":0},"Timer4":{"Arm":0,"Mode":0,"Time":"00:00","Window":0,"Days":"0000000","Repeat":0,"Output":1,"Action":0}}}
07:47:27 MQT: stat/sonoff/RESULT = {"Timers2":{"Timer5":{"Arm":0,"Mode":0,"Time":"00:00","Window":0,"Days":"0000000","Repeat":0,"Output":1,"Action":0},"Timer6":{"Arm":0,"Mode":0,"Time":"00:00","Window":0,"Days":"0000000","Repeat":0,"Output":1,"Action":0},"Timer7":{"Arm":0,"Mode":0,"Time":"00:00","Window":0,"Days":"0000000","Repeat":0,"Output":1,"Action":0},"Timer8":{"Arm":0,"Mode":0,"Time":"00:00","Window":0,"Days":"0000000","Repeat":0,"Output":1,"Action":0}}}
07:47:27 MQT: stat/sonoff/RESULT = {"Timers3":{"Timer9":{"Arm":0,"Mode":0,"Time":"00:00","Window":0,"Days":"0000000","Repeat":0,"Output":1,"Action":0},"Timer10":{"Arm":0,"Mode":0,"Time":"00:00","Window":0,"Days":"0000000","Repeat":0,"Output":1,"Action":0},"Timer11":{"Arm":0,"Mode":0,"Time":"00:00","Window":0,"Days":"0000000","Repeat":0,"Output":1,"Action":0},"Timer12":{"Arm":0,"Mode":0,"Time":"00:00","Window":0,"Days":"0000000","Repeat":0,"Output":1,"Action":0}}}
07:47:27 MQT: stat/sonoff/RESULT = {"Timers4":{"Timer13":{"Arm":0,"Mode":0,"Time":"00:00","Window":0,"Days":"0000000","Repeat":0,"Output":1,"Action":0},"Timer14":{"Arm":0,"Mode":0,"Time":"00:00","Window":0,"Days":"0000000","Repeat":0,"Output":1,"Action":0},"Timer15":{"Arm":0,"Mode":0,"Time":"00:00","Window":0,"Days":"0000000","Repeat":0,"Output":1,"Action":0},"Timer16":{"Arm":0,"Mode":0,"Time":"00:00","Window":0,"Days":"0000000","Repeat":0,"Output":1,"Action":0}}}
07:47:29 MQT: tele/sonoff/STATE = {"Time":"2018-09-30T07:47:29","Uptime":"0T00:00:16","Vcc":2.747,"POWER":"ON","Wifi":{"AP":1,"SSId":"Wireless","BSSId":"DE:AD:00:00:BE:EF","Channel":3,"RSSI":72}}
07:48:14 SRC: Timer
07:48:14 MQT: stat/sonoff/RESULT = {"POWER":"OFF"}
07:48:14 MQT: stat/sonoff/POWER = OFF
07:48:14 CFG: Saved to flash at 3FB, Count 17, Bytes 3584

@andrethomas
Copy link
Contributor

@ascillato Ok, will try a sunset

@andrethomas
Copy link
Contributor

Sunset is 19h32, so configured this

image

Output when setting is done

07:53:00 HTP: Save configuration
07:53:00 MQT: Timers 1,0xC87F82D0,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000,0x00000000
07:53:00 HTP: Configuration
07:53:01 CFG: Saved to flash at 3FA, Count 18, Bytes 3584

Now let me enable time travel on the NTP server to 19h31 and reboot the sonoff

Output log:


00:00:00 APP: Boot Count 7
00:00:00 SRC: Restart
00:00:00 Project sonoff Sonoff (Topic sonoff, Fallback DVES_10373D, GroupTopic sonoffs) Version 6.2.1.10-2_3_0
00:00:00 CFG: Saved to flash at 3F9, Count 19, Bytes 3584
00:00:00 WIF: Attempting connection...
00:00:00 WIF: Patch issue 2186
00:00:00 WIF: Connecting to AP1 Wireless in mode 11N as sonoff-5949...
00:00:06 WIF: Connected
00:00:06 DNS: Initialized
00:00:06 HTP: Web server active on sonoff-5949.local with IP address 192.168.42.123
00:00:07 MQT: Attempting connection...
00:00:07 MQT: Connected
00:00:07 MQT: tele/sonoff/LWT = Online (retained)
00:00:07 MQT: cmnd/sonoff/POWER = 
00:00:07 MQT: Subscribe to cmnd/sonoff/#
00:00:07 MQT: Subscribe to cmnd/sonoffs/#
00:00:07 MQT: Subscribe to cmnd/DVES_10373D/#
00:00:07 MQT: tele/sonoff/INFO1 = {"Module":"Generic","Version":"6.2.1.10","FallbackTopic":"DVES_10373D","GroupTopic":"sonoffs"}
00:00:07 MQT: tele/sonoff/INFO2 = {"WebServerMode":"Admin","Hostname":"sonoff-5949","IPAddress":"192.168.42.123"}
00:00:07 MQT: tele/sonoff/INFO3 = {"RestartReason":"Software/System restart"}
00:00:07 MQT: stat/sonoff/RESULT = {"POWER":"OFF"}
00:00:07 MQT: stat/sonoff/POWER = OFF
00:00:08 APP: (UTC) Sun Sep 30 17:31:14 2018, (DST) Sun Mar 25 02:00:00 2018, (STD) Sun Oct 28 03:00:00 2018
19:31:18 CMD: timer
19:31:18 SRC: Serial
19:31:18 RSL: Group 0, Index 1, Command TIMER, Data 
19:31:18 MQT: stat/sonoff/RESULT = {"Timer1":{"Arm":1,"Mode":2,"Time":"-00:00","Window":0,"Days":"1111111","Repeat":1,"Output":1,"Action":1}}
19:31:22 MQT: tele/sonoff/STATE = {"Time":"2018-09-30T19:31:22","Uptime":"0T00:00:16","Vcc":2.747,"POWER":"OFF","Wifi":{"AP":1,"SSId":"Wireless","BSSId":"DE:AD:00:00:BE:EF","Channel":3,"RSSI":72}}
19:32:07 SRC: Timer
19:32:07 MQT: stat/sonoff/RESULT = {"POWER":"ON"}
19:32:07 MQT: stat/sonoff/POWER = ON
19:32:07 CFG: Saved to flash at 3F8, Count 20, Bytes 3584

@arendst @ascillato I must be really bored to be doing this ridiculous testing ;)

@gitMichael
Copy link
Author

Thank you very much for your patience. I can live with the -00:00 solution.
But even though I'm only two and have a funny name and time is not an exact science (@andrethomas ), do me a favor and take also a look at line 212. I'm afraid 24:00 is usualy not a real time of day.
Should it be better: if (timeBuffer > 1439) { ?
Similar problem of the same mistake. Long live that unsigned int...
@andrethomas The problems with scientific time calculation are the unusual limits and overflows, not random values.

@ascillato2
Copy link
Collaborator

ascillato2 commented Sep 30, 2018 •
edited
Loading

Closing issue as now it is fixed.

Thanks everyone for testing and searching to solve the problem.

@gitMichael, thanks for the report, but please, next time try to be kind in order to let us focus only in the report and not in an incorrect way of talking to people.

If you need something else, please find us in the Tasmota Discord Chat at: https://discord.gg/Ks2Kzd4

Repository owner locked as resolved and limited conversation to collaborators Sep 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
awaiting feedback Action - Waiting for response or more information bug Type - Confirmated Bug fixed Result - The work on the issue has ended
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@andrethomas @arendst @Jason2866 @meingraham @ascillato @ascillato2 @gitMichael