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

Fix/ocpp initial status 3 #18468

Open
wants to merge 4 commits into
base: fix/ocpp-initial-status-3
Choose a base branch
from

Conversation

mfuchs1984
Copy link
Contributor

@andig I took your branch fix/ocpp-initial-status-3 made to resolve #15677 and performed some debugging to resolve #18443. It was successfully initializing with the eProwallbox that does not support remote trigger after my changes.

Basically, the changes are:

  1. Fix a panic in 8adb8fe. The original code was accessing cs.regs[chargePoint.ID()].cp which was not existing yet.
  2. Change the order of caching status and calling cp.OnStatusNotification(request) in 67fe269. Without this change, the caching never happened since in the moment the function is called, cs.ChargepointByID(id) already succeeds, only the connector is not available yet. With the change it will now always do the caching, but I think this is no issue since the cached status is only used during init. Commit 5c3fb4a fixed a mutex deadlock I introduces with the change.
  3. Use the cached status when the charger does not support RemoteTrigger in 236ed2a. In addition, I guarded all other calls to TriggerMessageRequest with HasRemoteTriggerFeature

@andig andig added the devices Specific device support label Jan 29, 2025
@andig
Copy link
Member

andig commented Jan 29, 2025

Connector tests are panicking now- that‘s unexpected?

@mfuchs1984
Copy link
Contributor Author

Connector tests are panicking now- that‘s unexpected?

What exactly is panicking? I didn't have issues, just the panic I resolved.

@andig
Copy link
Member

andig commented Jan 29, 2025

See below

@mfuchs1984
Copy link
Contributor Author

Ah danke, schaue ich mir an.

@mfuchs1984
Copy link
Contributor Author

Sind das die gleichen failures wie im source branch? #16908
Kann die Details dort leider nicht einsehen.

Wenn ja, dann denke ich, es liegt daran, dass instance im Test nicht instanziiert wird. WithConnectorStatus greift auf cs.mu zu, aber es gibt kein cs Objekt. Im normalen Ablauf passiert das meinem Verständnis nach hier

instance = &CS{

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devices Specific device support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants