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

Fix inconsistant vehicle publishing #11675

Merged
merged 6 commits into from
Jan 15, 2024
Merged

Fix inconsistant vehicle publishing #11675

merged 6 commits into from
Jan 15, 2024

Conversation

naltatis
Copy link
Member

@naltatis naltatis commented Jan 14, 2024 •
edited
Loading

fixes: #11666

I made a few simplifications to the code in order to make get to the root cause.

  • 🗑️ removed vehiclePresent since its redudant to the existance of vehicleName
  • 🗑️ removed vehicleTitle since it should be lookedup in vehicles via vehicleName
  • 🪲 some minor UI code cleanups and error handlings

The issue seems to be out-of-sync publishing. The .publish() calls are executed in the right order but are sometimes processed out of sync.

Reproduce:

go run main.go --log TRACE` (using `demo.yaml`)

In one time of three the error occures on my machine resulting in showing "Guestvehicle" instead of "Model 3" or "e Golf".
I've added 🔊 log entries to highlight the issue.

[lp-2  ] DEBUG 2024/01/14 12:24:04 pv timer inactive
[lp-2  ] DEBUG 2024/01/14 12:24:04 guard timer inactive
[cache ] TRACE 2024/01/14 12:24:04 lp-2/limitEnergy: 20
[cache ] TRACE 2024/01/14 12:24:04 lp-2/planEnergy: 0
>>> [lp-2  ] INFO 2024/01/14 12:24:04 🔊 vehicle: remove 2
[lp-2  ] INFO 2024/01/14 12:24:04 vehicle updated: unknown -> weißes Model 3
>>> [lp-2  ] INFO 2024/01/14 12:24:04 🔊 vehicle: name: vehicle_2, capacity: 80
[cache ] TRACE 2024/01/14 12:24:04 lp-2/priority: 0
[cache ] TRACE 2024/01/14 12:24:04 lp-2/title: Garage
[cache ] TRACE 2024/01/14 12:24:04 lp-2/mode: pv
[cache ] TRACE 2024/01/14 12:24:04 lp-2/minCurrent: 6
[cache ] TRACE 2024/01/14 12:24:04 lp-2/maxCurrent: 16
[cache ] TRACE 2024/01/14 12:24:04 lp-2/phasesEnabled: 0
[cache ] TRACE 2024/01/14 12:24:04 lp-2/enableThreshold: 0
[cache ] TRACE 2024/01/14 12:24:04 lp-2/disableThreshold: 0
[site  ] WARN 2024/01/14 12:24:04 interval <30s can lead to unexpected behavior, see https://docs.evcc.io/docs/reference/configuration/interval
[site  ] DEBUG 2024/01/14 12:24:04 ----
[cache ] TRACE 2024/01/14 12:24:04 lp-2/phaseRemaining: 0s
[cache ] TRACE 2024/01/14 12:24:04 lp-2/phasesConfigured: 0
[cache ] TRACE 2024/01/14 12:24:04 lp-2/pvAction: inactive
[cache ] TRACE 2024/01/14 12:24:04 lp-2/phaseAction: inactive
[cache ] TRACE 2024/01/14 12:24:04 lp-2/phasesActive: 3
[cache ] TRACE 2024/01/14 12:24:04 lp-2/guardAction: inactive
[cache ] TRACE 2024/01/14 12:24:04 lp-2/pvRemaining: 0s
[cache ] TRACE 2024/01/14 12:24:04 lp-2/guardRemaining: 0s
[cache ] TRACE 2024/01/14 12:24:04 lp-2/chargerFeatureIntegratedDevice: false
[cache ] TRACE 2024/01/14 12:24:04 lp-2/chargerIcon:
[cache ] TRACE 2024/01/14 12:24:04 lp-2/chargerFeatureHeating: false
>>> [cache ] TRACE 2024/01/14 12:24:04 lp-2/vehicleName: vehicle_2
[cache ] TRACE 2024/01/14 12:24:04 lp-2/vehicleOdometer: 0
[cache ] TRACE 2024/01/14 12:24:04 lp-2/vehicleIcon:
>>> [cache ] TRACE 2024/01/14 12:24:04 lp-2/vehicleName:
[cache ] TRACE 2024/01/14 12:24:04 lp-2/vehicleCapacity: 0
[cache ] TRACE 2024/01/14 12:24:04 lp-2/vehicleIcon:
[cache ] TRACE 2024/01/14 12:24:04 lp-2/effectiveMaxCurrent: 16

You can see, that 🔊 vehicle: remove and 🔊 vehicle: name are done in correct order with correct data.
Updating the publish state is done async and in wrong order lp-2/vehicleName: vehicle_2 and lp-2/vehicleName:

@andig any ideas why?

@andig
Copy link
Member

andig commented Jan 14, 2024 •
edited
Loading

Wir haben vor einiger Zeit- allerdings in 0.123.2- einen Deadlock behoben: #11244. Damit werden Werte asynchron per Go Routine gepublished wenn die eigentliche Queue verstopft ist. Es wäre interessant, ob das hier so ein Fall ist?

Statt via Go Routine wäre es besser, eine globale Queue dafür zu nutzen- dann bliebe die Reihenfolge erhalten. Die Komplexität mit der Queue ist, dass es einen Mechanismus braucht, der aus der Queue in den fall wieder freien Channel pusht.

Der Umbau hier ist aber in jedem Fall sinnvoll!

@andig andig added the bug Something isn't working label Jan 14, 2024
@andig
Copy link
Member

andig commented Jan 14, 2024

@naltatis könntest Du den Vorschlag mal ausprobieren? Das sollte jetzt ein Channel ohne Größenlimit sein statt asynchroner Goroutinen und damit die Reihenfolge sicherstellen.

@andig andig marked this pull request as ready for review January 14, 2024 17:23
andig
andig reviewed Jan 14, 2024
View reviewed changes
core/loadpoint.go Outdated Show resolved Hide resolved
core/loadpoint_vehicle.go Outdated Show resolved Hide resolved
core/loadpoint_vehicle.go Outdated Show resolved Hide resolved
@andig andig marked this pull request as draft January 14, 2024 17:34
@naltatis naltatis marked this pull request as ready for review January 15, 2024 07:49
@naltatis
Copy link
Member Author

Sieht gut aus. Ich kann den Bug nun nicht mehr reproduzieren. 🙏

@andig andig merged commit 4fa0155 into master Jan 15, 2024
6 checks passed
@andig andig deleted the fix/broken_vehicle_publishing branch January 15, 2024 07:54
@andig
Copy link
Member

andig commented Jan 15, 2024

Klasse, danke für den schneller Test! Man merkt mal wieder: wenn eine Änderung sich nicht perfekt anfühlt lieber nicht machen...

@VolkerK62
Copy link
Contributor

🗑️ removed vehicleTitle since it should be lookedup in vehicles via vehicleName

kann das in Bezug auf Messaging (vehicleTitle) "Folgeschäden" haben?

@andig
Copy link
Member

andig commented Jan 15, 2024 •
edited
Loading

Ja :(. Ich mache Folgeissue auf: #11705

@andig andig mentioned this pull request Jan 15, 2024
Closed
@naltatis naltatis mentioned this pull request Jan 17, 2024
Closed
naltatis added a commit that referenced this pull request Jan 25, 2024
@naltatis
Fix inconsistent vehicle publishing (#11675)
737ce0b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andig andig andig left review comments

Assignees

@andig andig

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fahrzeug wird nicht mehr erkannt
3 participants
@naltatis @andig @VolkerK62