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.

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

Test report for RC3 #19

Closed
KipK opened this issue Jan 5, 2023 · 46 comments
Closed

Test report for RC3 #19

KipK opened this issue Jan 5, 2023 · 46 comments

Comments

@KipK
Copy link
Owner

KipK commented Jan 5, 2023

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
Copy link

fhteagle commented Jan 15, 2023

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
Copy link
Owner Author

KipK commented Jan 15, 2023

@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
Copy link
Owner Author

KipK commented Jan 17, 2023

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

@fhteagle
Copy link

fhteagle commented Jan 18, 2023

At release:
OpenEVSE/openevse_esp32_firmware@d98e8b7

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
Copy link
Owner Author

KipK commented Jan 18, 2023

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
Copy link
Owner Author

KipK commented Jan 18, 2023

Vivaldi above, and opera here :

Screenshot_20230118-091228

@KipK
Copy link
Owner Author

KipK commented Jan 18, 2023

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
Copy link

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
Copy link

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
Copy link
Owner Author

KipK commented Jan 19, 2023

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
Copy link

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
Copy link
Owner Author

KipK commented Jan 21, 2023

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
Copy link

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
Copy link
Owner Author

KipK commented Jan 22, 2023

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

@fhteagle
Copy link

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
Copy link

fhteagle commented Jan 27, 2023

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
Copy link
Owner Author

KipK commented Jan 28, 2023

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
Copy link
Contributor

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
Copy link
Owner Author

KipK commented Jan 28, 2023

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
Copy link

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

firstof9/openevse#120

@fhteagle
Copy link

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

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

@KipK
Copy link
Owner Author

KipK commented Jan 28, 2023

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

@fhteagle
Copy link

fhteagle commented Jan 29, 2023

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
Copy link
Owner Author

KipK commented Jan 29, 2023

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

@jeremypoulter
Copy link
Contributor

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
Copy link
Owner Author

KipK commented Jan 29, 2023

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
Copy link

fhteagle commented Feb 7, 2023

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
Copy link
Owner Author

KipK commented Feb 7, 2023

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

@fhteagle
Copy link

fhteagle commented Feb 7, 2023

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
Copy link
Owner Author

KipK commented Feb 7, 2023

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

@fhteagle
Copy link

fhteagle commented Feb 7, 2023

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

@KipK
Copy link
Owner Author

KipK commented Feb 7, 2023

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
Copy link
Owner Author

KipK commented Feb 7, 2023

my PR was merged yesterday ( OpenEVSE/openevse_esp32_firmware#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
Copy link

fhteagle commented Feb 7, 2023

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

@KipK
Copy link
Owner Author

KipK commented Feb 7, 2023

Nope It's a wifi fw feature only

@KipK
Copy link
Owner Author

KipK commented Feb 7, 2023

good to go now . Thanks for your help

@fhteagle
Copy link

fhteagle commented Feb 7, 2023

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
Copy link

fhteagle commented Feb 7, 2023

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

@fhteagle
Copy link

fhteagle commented Feb 7, 2023

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
Copy link
Owner Author

KipK commented Feb 7, 2023

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
Copy link
Owner Author

KipK commented Feb 7, 2023

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
Copy link
Owner Author

KipK commented Feb 7, 2023

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
Copy link
Owner Author

KipK commented Feb 7, 2023

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

@fhteagle
Copy link

fhteagle commented Feb 15, 2023

@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
Copy link
Owner Author

KipK commented Feb 15, 2023

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
Copy link

fhteagle commented Feb 17, 2023

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

@KipK KipK closed this as completed Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants