Manual setting of time requires a change of date to work

fhteagle commented 1 year ago

Trying out your GUI against my live 7.1.3 / 4.1.5 OpenEVSE, I found something that is not too intuitive:

If you switch from NTP to manual time, then click the date/time field to pull up the popup:

Current time is not loaded into the popup, instead it comes up as 00:00
If you change only the time then click close, the date/time field is not populated.

If you do change the date, then the date/time field is populated.

(If it is not helpful for me to be testing / trying while you are actively dev'ing, let me know, this can of course wait until later)

KipK commented 1 year ago

I was working on it while you tested I think.

Can you refresh your git ?

fhteagle commented 1 year ago

Same behavior at master commit 8b65b80. No relevant error messages from npm, nor in browser js console. Using Vivaldi (chromium based) browser on mobile and desktop, have not tried any other browsers.

KipK commented 1 year ago

strange I have fixed that today. Fore refresh with Shift Refresh, or clean your cache , I haven't tested other browsers than chrome & firefox yet.

KipK commented 1 year ago

You're testing with npm run dev ?

KipK commented 1 year ago

screen Do you see the modal like this ( It was different before ) ? I have tested on Vivaldi and everything works here. It's probably a cache problem

fhteagle commented 1 year ago

After deleting the local directory, re-cloning your repo (with submodules) at 2316 UTC, latest in git log is:

commit d3b288e91d796aae34403c7d3c2ed5d7323d0bb3 (HEAD -> master, origin/master, origin/HEAD) Author: KipK hotlobster75@gmail.com Date: Thu Nov 10 00:04:46 2022 +0100

force blur on button clic

then npm install, npm run dev, The modal comes up without the current time selected. Checked in Vivaldi, Chromium, and Firefox.

Screenshot_20221109_161407

Let me know what else would be helpful to troubleshoot.

KipK commented 1 year ago

I have no clue yet. Have you set the .env file too ?

KipK commented 1 year ago

no error in the chrome developper console?

fhteagle commented 1 year ago

.Okay we'll figure it out.

Contents of .env:

VITE_OPENEVSEHOST = "openevse.local"
VITE_REMOTEHOST = "false"

openevse.local does resolve to my physical openevse unit. Correct realtime data is pulled from it. Not sure what the second option does?

fhteagle commented 1 year ago

This is the only thing in developer console:

The keyword 'slider-vertical' specified to an 'appearance' property is not standardized. It will be removed in the future.

KipK commented 1 year ago

I did like you fetched fresh repo, etc, and it still works here... :)

KipK commented 1 year ago

I'm using platformio btw

fhteagle commented 1 year ago

Computers: where the definition of insanity does not apply. You can do the same thing over and over again and get different results.

Okay instead of chasing this around in circles, how about I check it again when the GUI is "done"?

KipK commented 1 year ago

Just an idea, what time zone are you?

fhteagle commented 1 year ago

America/Denver , aka US mountain time, currently UTC -7

KipK commented 1 year ago

can you post a pic of how is displayed the time in the input field ?

fhteagle commented 1 year ago

Okay, not sure what information you needed for what stage of the time setting process, so I am going to give too many rather than too few.

Tested at commit b29ea75e1e9a331bf565a09523f13c1bd32a8d51 just now:

How it comes up on first load, with NTP as time setting method, with correct current time in the textbox: Screenshot_20221110_111304

Change to manual time setting in dropdown, still correct date and time in the textbox: Screenshot_20221110_111345

Click textbox, modal comes up with today's date in an empty circle, and 00:00 time selected. The circle around the date of the 2nd was from a mouseover event that did not clear during screenshot process, please disregard it. Screenshot_20221110_111417

If I only set the time in the modal (do not click any date, date remains in an unfilled circle symbol), then click close, the textbox comes back blank . The textbox will fill back in after some seconds, maybe 10 or 15, though. Screenshot_20221110_111548

If I only click on the date (date circle is filled in), but do not change the time, the textbox is filled with the selected date and 00:00 for the time Screenshot_20221110_112937

If I click the date in the modal (date circle is filled in), and set the time, the textbox has the correct value from the modal: Screenshot_20221110_111500

I tested this using my normal time zone, and about 5 other named or GMT+/- time zones selected. In each case, the behavior was exactly the same.

KipK commented 1 year ago

Thanks I've found the problem so :)

Will be fixed.

fhteagle commented 1 year ago

Okay great, let me know and I will re-test

KipK commented 1 year ago

can you git pull & retry now ?

KipK commented 1 year ago

oups it introduced a new bug.. will fix tomorrow

edit: fixed now

fhteagle commented 1 year ago

Textbox and modal now function as expected at commit 06831411848722df5da000c5a16d3effff37ff36

However, the set manual time operation no longer completes. After clicking "Set Time" button, loading chase circle stays in the button continously (on desktop) / button stays filled in blue on mobile. Confirmed that /status endpoint is also not updated with the new time.

"Use Current Time" button appears to also fail to complete the time update.

fhteagle commented 1 year ago

One possible further bug: the new GUI does not seem to pick up on the "sntp_enabled":false from the /config endpoint (this is what tells the OpenEVSE / GUI what time mode is in use, correct?!?) . I.e., the new GUI always shows me NTP as the time method enabled, even after I have forced it to manual in the 4.1.5 GUI ........

KipK commented 1 year ago

Can you try again ? I have added the sntp_enabled setting now ( I've missed this one, thanks )

Also it should submit time now, but there's still the bug on openevse side with manual time.

fhteagle commented 1 year ago

Testing at commit fcc74f87ac9153316005c6aa63031d75a26bfd00:

New GUI correctly indicates time method from sntp_enabled config variable at page load.
I am seeing inconsistent results when clicking the "Set Time" button. The buttons react as expected only if you click the textbox, then Set Time from Browser, then Set Time. Set Time button gets stuck on the loading circle when manually inputting a time via the modal, then clicking close, then clicking Set Time.

However, time indicated at /status endpoint is not actually updated in either case.

I saw the "Use current time" button got moved to the modal as "Set Browser Time". I think I preferred having that button on the page itself.

Let me know when it is ready for more testing.

KipK commented 1 year ago

I've completely removed the fancy time picker. This should be more consistant. Also fixed the submit bug introduced

fhteagle commented 1 year ago

I like the dropdown time picker better than the modal! Works well with keyboard, mouse, touchscreen, etc.

Confirmed manual time setting buttons are working and time update is reaching /status endpoint in commit cccdb8d4934974b0c49597cc4345eb0cd4f15607 .

Of course the time handling bug as documented in https://github.com/OpenEVSE/ESP32_WiFi_V4.x/issues/467 and https://github.com/OpenEVSE/ESP32_WiFi_V4.x/issues/302 remains

Thanks for the fixes

fhteagle commented 1 year ago

One more little tiny thing:

When you select time mode: manual, the checkbox appears and it becomes manual time in /config and /status right away
When you select time mode: ntp, the checkbox appears on GUI, sntp_enabled: becomes true in /config, but the time is not actually updated until you click Set Time. I think many users will not remember to click "Set Time", and then think this is a bug.............

Can you change it so as soon as you select NTP time mode, the correct time is pulled from NTP server as if "Set Time" was clicked?

KipK commented 1 year ago

done

fhteagle commented 1 year ago

Found another silly gotcha...

If GUI client A changes the time mode, the clock at upper right and the textbox for time setting on GUI client B on another browser or device is not updated.

KipK commented 1 year ago

you have to wait for the websocket to send updated time for the top time display , it comes each 30sec. time setting textbox for now won't update, as an update can occur just after you've set the time in the time picker. I've disabled it. It is the case in the current UI you have to be fast enough or time change in the field. I'll add the pick current time button to adress this issue. Btw, we can update the time while NTP is set.

I'm still looking for a good algorithm, perhaps this one should fit all the cases: if ntp: allow time update if manual, if input field as not been manually changed && new ime has not been sent, block time update, then after time save, allow time update.

fhteagle commented 1 year ago

Makes sense about websocket, but I think I am running into cases where status block does not update. Splitting to new issue: https://github.com/KipK/openevse-gui-v2/issues/8

If I am reading your algorithm working correctly: Textbox self-updates during NTP mode but is locked from user entry. Textbox unlocks for user entry but does not self-update in manual mode. Makes sense to me!

KipK commented 1 year ago

I've pushed somehting like this. It stills update on manual mode until you focus the field , update will resume if time is submitted

KipK / openevse-gui-v2

Manual setting of time requires a change of date to work #2