KipK / openevse-gui-v2

OpenEVSE WiFi Gateway User Interface (V2)
BSD 2-Clause "Simplified" License
5 stars 2 forks source link

Test report for RC3 #19

Closed KipK closed 1 year ago

KipK commented 1 year ago

I have pushed RC3 here : https://github.com/KipK/openevse-gui-v2/releases/tag/1.0.0-rc3

There's a binary build for openevse_wifi_v1 module that can be installed safely.

@fhteagle, @jeremypoulter If you can give a look.

fhteagle commented 1 year ago

So I had not been active with testing as my unit was completely non-functional on wifi. Seemed to be a problem with 4.1.5, but I finally got a chance to be stubborn and power cycle it a few dozen times, so I could get 4.1.7 loaded onto it. We'll see if it behaves from here on out, wish me luck. Testing things now...

Typo on Configuration tab: "Developer" instead of "Developper".

KipK commented 1 year ago

@fhteagle, there's now daily builds with new ui here: https://github.com/OpenEVSE/ESP32_WiFi_V4.x/releases/tag/v2_gui?fbclid=IwAR1Ty7Wr735bHMQkGC1wAZp0_K4FcNVNFThaVxQ9_hIbti59UYKUKpWAVb8

Do not put 4.1.7 it has a bug with MQTT making wifi drops and more. Use daily builds instead for now

KipK commented 1 year ago

@fhteagle , I have fixed the time setting issue, if you can test that now everything is workoing ok for you

fhteagle commented 1 year ago

At release: https://github.com/OpenEVSE/ESP32_WiFi_V4.x/commit/d98e8b7d4d6c8d6b595657b24e12a247801aecbf

Agreed that your nightly is more stable than the official 4.1.7 . Wifi is working way better than it has since about 4.1.3 or so. Any chance we can get the version incremented in the UI so we can see we are not on "stock" 4.1.7 anymore? A version string like 4.1.7-rc3-d98eb7d4 would help to assure the user that their uploaded nightly build firmware did in fact get applied.

Visual glitch: on Vivaldi on Android, the top part of the status bar is cutoff. The two icons at the top left for charge status and vehicle connection status, the OpenEVSE logo, and the time block at the upper right all cannot be seen. Firefox Focus, stock Chrome browser on Android display as expected.

On Opera on Android, the "Connection Error" banner shows all the time, even when tabs load with correct data.

On the history tab, I cannot see a way to read the content of an Error event line. For example, I had a false "No Ground" detection a few days ago. I see the red caution triangle with ! icon, but no way to see that that history tab line is a No Ground event.

I did test time setting. Manual seems to work reliably, but the time jumps to UTC when the Manual option is first selected. Local time option works reliably for me. Setting UTC time zone when NTP is selected does technically work, but feels a bit of a hack to me. It works, just could be a bit more slick. If time zone is not going to be retained, maybe ask the user the correct timezone immediately, or focus and highlight the timezone entry box?

I will keep exploring the UI and see if I can find anymore issues. Thanks.

KipK commented 1 year ago

Can you try the latest daily build for V2 ?

I think some of your comments are already fixed. I have tested on Vivaldi and have no such problem

Screenshot_20230118-085254

KipK commented 1 year ago

Vivaldi above, and opera here :

Screenshot_20230118-091228

KipK commented 1 year ago

Also you'll notice there's UX difference between some forms. I have started to rework them, like HTTP ,MQTT m, Time and RFID. They will all be done the same way without save button ( but only if really necessary )

edit: I have just commited the error code display on Logs and Status bar, thanks for remind me it

fhteagle commented 1 year ago

Updated to the nightly build from 5 hours ago.

Tooltips on icons in history works, merci beaucoup. However, I just noticed that the date/time column of history appears truncated:

Screenshot_20230118_084552

Using OEM Android 12 on OnePlus 7T:

Still seeing the truncated status block in Vivaldi 5.6.

Screenshot_2023-01-18-08-32-31-91_d365b52accad0f47adbc08c16219827d

Same with the connection error banner on Opera 72.5.3767.69342.

Screenshot_2023-01-18-08-54-03-24_4641ebc0df1485bf6b47ebd018b5ee76

I can start checking other phones if we need to track these display bugs down further.

On MQTT settings, the "Show" tickbox does not reveal the password.

fhteagle commented 1 year ago

Also, on the history tab, rightmost column, can we get the Temp to be a consistent precision? I really do not care if it is an integer only (17) or one decimal place (17.2), but having them all the same precision makes the column look cleaner.

KipK commented 1 year ago

Thanks @fhteagle , I think I've covered all your issue but the Opera websocket disconnection error I can't reproduce. If you can catch up something on developper console log ...

Edit: I've found it, it's the VPN prolly activated in your Opera, seems to cause trouble with websockets . Edit2: it seems Opera VPN ( that is not , it's just a proxy ) is routing all wss:// traffic to it. And has some known problems with wss headers So the webpage can't call with local network with something else than http ) Opera fake VPN needs to be desactivated so

I have posted a bug report on Opera team. Btw, why using Opera in 2023?

fhteagle commented 1 year ago

I tested again on an S9 with Android 10. Could not reproduce either the Vivaldi or Opera bugs on that phone. So both UI display glitches may be a one off issue with my OnePlus 7T.

I have a friend who sends me all kinds of current events and political stuff, some of it useful alternative news, but some of it is straight up crazy conspiracy theory stuff. I use Opera Privacy Tabs to load the links from him, so it does not get associated with my usual digital footprint lol. Just thought I would try the GUIv2 out there as well (not in a privacy tab of course), just for testing purposes. Agreed that is not a high market share browser and probably not worth spending too much time worrying about.

KipK commented 1 year ago

Hey

I have made a dirty hack to handle the OpenEVSE time bug can you try latest build? I reset the TZ to UTC to manual and save the previous tz setting on browser local storage . It restores it when ntp is selected . Off course first time won't works and will display UTC. There's still some quick glitche appearing on time displayed at save but it corrects itself fast as it refetch /status just after and overwrite the first time event.

I'll remove that when @jeremypoulter solve it on api

I have also removed the set to browser time, it nows automatically set to local time when you select manual,

fhteagle commented 1 year ago

Dirty hack seems to operate as expected from every browser I checked. Agreed that the first status block update with the completely random time is odd, possibly confusing for the user. Is there a way to mask the first wrong time refresh and keep it from displaying? Perhaps replace with "Updating time...."?

KipK commented 1 year ago

I could make the websocket ignore time property for the first receive time update. But that's a really dirty hack 😁

fhteagle commented 1 year ago

After GUI-triggered restart of EVSE and Wifi board, time gets stuck at UTC until I do a manual change of time and back to NTP. Probably a result of the "dirty hack". Not sure it is worth fixing, though, maybe just wait for the API to get fixed properly....

fhteagle commented 1 year ago

Other really really really minor thing, I am testing Homeassistant controlling the OpenEVSE through an integration that uses primarily HTTP API. When the "max amps" is selected in HomeAssistant, sometimes the GUI does not update the max amps on the main charging tab, even after a few of the 30 second update ticks have gone by. If it does update, I get an extra zero in the front of single digit amp values. After refreshing the browser shows the updated value from HomeAssistant automation 100% of the time though. So I think it is just the GUI not getting an update somehow.

Note to self: do more digging to see if the integration is updating the wrong soft vs hard max amps variable..........

KipK commented 1 year ago

I wonder why the max_current value is modifiable through HA. This is a one time setup. Shouldn't they adjust charge_current instead?

jeremypoulter commented 1 year ago

Home assistant should probably expose both as you could use it to implement many use cases, however the charge_current should be the main one that shows up by default, the max current should probably be hidden by default.

KipK commented 1 year ago

If HA use max_current_soft from /config, it then should display in the UI slider max pos as it should download automatically the new /config if there's a change.

fhteagle commented 1 year ago

This is what I was trying to confirm about in this issue over there:

https://github.com/firstof9/openevse/issues/120

fhteagle commented 1 year ago

Retested GUIv2 release from 3 hours ago, the following bug remains:

On MQTT settings, the "Show" tickbox does not reveal the password.

KipK commented 1 year ago

it's normal, it only reveal while you have typed it yourself. The api doesn't send the password so it stays hidden

fhteagle commented 1 year ago

Okay, suggestions to handle that behavior more elegantly:

a. hide the Show tickbox if there is nothing to show yet, and/or b. instead of "password dots", fill the password textbox with "unchanged" placeholder instead

I think both of these would more clearly communicate that the GUI does not know what the password is, so do not expect to be able to show it...

Just grabbed the latest nightly build, planning on going through everything I can test on my setup very thoroughly today.

KipK commented 1 year ago

Thanks for the input. I consider hiding the tickbox yes, it's on my list.

jeremypoulter commented 1 year ago

For the v1 UI we show nothing when the show password is selected on an existing password. We do this because it is likely that the user wants to enter a new password

KipK commented 1 year ago

I have considered that,but then we don't know a password has been set before. the change with no checkbox is on latest builds

fhteagle commented 1 year ago

I am not seeing any other showstopper bugs in GUI v2 after a couple of weeks of pretty thorough testing on the nightlies, trying to break things, etc. Great work @KipK on this.

KipK commented 1 year ago

You can download the latest build, there's many changes and it needs to be tested ;)

fhteagle commented 1 year ago

I've been pulling each new build as it shows up here: https://github.com/OpenEVSE/ESP32_WiFi_V4.x/releases/tag/v2_gui

Or is there a different branch that needs more testing?

KipK commented 1 year ago

No it is this one. Have you tested the new Limit engine?

fhteagle commented 1 year ago

Just noticed the new limit box on the main tab. I will try it out and report back.

KipK commented 1 year ago

Hold on, I've just tested the generated fw on githb and it's not the good version, UI is ok but not the latest fw. I tell you when it's ok

KipK commented 1 year ago

my PR was merged yesterday ( https://github.com/OpenEVSE/ESP32_WiFi_V4.x/pull/535 ), I've pushed the Limit branch on UI too, but the UI_V2 branch of wifi fw has not been updated yet. So it generated a build with the Limit UI but without the feature on fw :) Should be fixed in a few

fhteagle commented 1 year ago

Will the limit engine controlled by Wifi firmware features require flashing the OpenEVSE firmware itself, or can I test against 7.1.3?

KipK commented 1 year ago

Nope It's a wifi fw feature only

KipK commented 1 year ago

good to go now . Thanks for your help

fhteagle commented 1 year ago

Found a quirk: If session is already in progress, and a time or energy limit is set below values already accumulated, the session goes straight to disable. This could be problematic for users and especially automations that are not aware of existing session amounts.

Potential solutions:

  1. Add limit amount to existing session values. I.E., if I have already been charging for 38 minutes, and select a 20 minute limit, add the two to actually establish a 58 minute limit (20 minutes remaining from time it is set), or
  2. Reject any limits that are less than current session accumulated values, and/or
  3. GUI popup prompt "Are you sure you want to set a limit less than already used"?, and/or
  4. Set the default value for the limit entry to be the current session value, so the user has to actively slide the input to less than current session value.

I am not sure which option(s) is/are most beneficial to users. I can make reasonable arguments in my head for each of them. How did GUI v1 and the old limit system handle this situation?

Also, rounding needed on time remaining figure:

Screenshot_20230207_112645

fhteagle commented 1 year ago

Also, is it not possible to set both a time and energy limit simultaneously? Is this by design?

fhteagle commented 1 year ago

With no schedule set, there is no "robot head" icon for automatic mode. With limit elapsed, neither charging active nor charging disabled is indicated on "Toggle Charge" icon row.

Screenshot_20230207_114821

Consider adding the robot head icon for automatic mode any time a limit is set as well.

KipK commented 1 year ago

Found a quirk: If session is already in progress, and a time or energy limit is set below values already accumulated, the session goes straight to disable. This could be problematic for users and especially automations that are not aware of existing session amounts.

Potential solutions:

  1. Add limit amount to existing session values. I.E., if I have already been charging for 38 minutes, and select a 20 minute limit, add the two to actually establish a 58 minute limit (20 minutes remaining from time it is set), or
  2. Reject any limits that are less than current session accumulated values, and/or
  3. GUI popup prompt "Are you sure you want to set a limit less than already used"?, and/or
  4. Set the default value for the limit entry to be the current session value, so the user has to actively slide the input to less than current session value.

I am not sure which option(s) is/are most beneficial to users. I can make reasonable arguments in my head for each of them. How did GUI v1 and the old limit system handle this situation?

Also, rounding needed on time remaining figure:

Screenshot_20230207_112645

hehe I was just correcting this display bug. I've pushed the fix, build in progress. I got the same question as you thereafter. Will think about it.

KipK commented 1 year ago

Also, is it not possible to set both a time and energy limit simultaneously? Is this by design?

yes for now only one limit. I thought about allowing multiple limits, but I've not found real use case but gadget ones. And it complicates the UX. However, the engine could handle that with few mod if well thought. For now it's already better than the evse fw limit. Let start with this.

KipK commented 1 year ago

With no schedule set, there is no "robot head" icon for automatic mode. With limit elapsed, neither charging active nor charging disabled is indicated on "Toggle Charge" icon row.

Screenshot_20230207_114821

Consider adding the robot head icon for automatic mode any time a limit is set as well.

Robot Icon means there's an active service controlling the states of the EVSE. If icon disappear it means there's no schedule, nor RFID, nor Divert, etc. Service that can booth enable and disable.

Limits is only made to disable the state. It's not part of automation features as it's still enabled when state is forced to active with Manual override.

But seeing the Enable/Disable buttons not selected , I get your problem. I have to think about that, but you're right, I should enable the robot Icon then. It just bugs me because it goes against what I've planned for it. Why not just set the button to disabled state and no robot ? Would this be confusing in UX perspective ?

KipK commented 1 year ago

Can you post this first & last issues on github ? I'm leaving for 10 days and don't want to forget.

fhteagle commented 1 year ago

@KipK -

Time limit display bug fix verified, thanks, looks much better.

"Why not just set the button to disabled state and no robot ?"

That makes sense to me too. If limit has been triggered, OpenEVSE is acting as disabled, so highlighting the disabled button with red background communicates that well.

I'm not sure which of my observed behavior comments you were wanting to have split to separate issue items. Happy to write-up the split, just need to know which ones.

KipK commented 1 year ago

I mean the 2 bugs / suggestions reports you've made but you can also create one for the simultaneous limits. It's easier to track them separated.

fhteagle commented 1 year ago

Okay, I split these into #27 and #28 .