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

TxProfile rejected #9766

Merged
merged 5 commits into from
Sep 21, 2023
Merged

TxProfile rejected #9766

merged 5 commits into from
Sep 21, 2023

Conversation

pjjjv
Copy link
Contributor

@pjjjv pjjjv commented Sep 6, 2023

For #8138 discussion. Do not commit.

@premultiply

This comment was marked as outdated.

@premultiply premultiply changed the title Workaround TxProfile rejected on ABB Terra AC Workaround TxProfile rejected Sep 7, 2023
@pjjjv
Copy link
Contributor Author

pjjjv commented Sep 9, 2023

Hold your horses. I'm still having reliability issues with this. I am still experimenting. Anyway, the problem with TxDefaultProfile is that it's simply not how this should work. If anybody else sets a TxProfile then that will overrule TxDefaultProfile and evcc can´t regulate anymore.

@pjjjv pjjjv changed the title Workaround TxProfile rejected TxProfile rejected Sep 16, 2023
@pjjjv
Copy link
Contributor Author

pjjjv commented Sep 16, 2023

This is a structural fix.

@andig
Copy link
Member

andig commented Sep 17, 2023

Gibts hier noch was zu tun? M.E. Bugfix bei ABB?

@pjjjv
Copy link
Contributor Author

pjjjv commented Sep 20, 2023

No, see #8138 . ABB firmware was according to spec after all. I would like to see my car charging properly tomorrow and if successful then I'll click this through to review for you to integrate. I don´t know how to make this a separate PR though, sorry.

@andig
Copy link
Member

andig commented Sep 20, 2023

Full ack after reading the spec. It seems that our OCPP library does not follow the spec in this regard.

@andig andig added the bug Something isn't working label Sep 20, 2023
@pjjjv
Copy link
Contributor Author

pjjjv commented Sep 21, 2023

Car still charging today. Looks ok to me. PLease integrate.

@pjjjv pjjjv marked this pull request as ready for review September 21, 2023 11:48
charger/ocpp.go Outdated Show resolved Hide resolved
@andig andig merged commit a3e0c8a into evcc-io:master Sep 21, 2023
@c-po
Copy link

c-po commented Sep 21, 2023

Thank you so much for troubleshooting and fixing this!

@pjjjv
Copy link
Contributor Author

pjjjv commented Sep 22, 2023

Hey, this will not work! The duplicate function was there for a reason (or you can solve it with an IF statement if you want). The transactionId can only be passed in setChargingProfile, but not in a RemoteStartTransaction (the transaction is still to be started, so the transactionId is unknown and the spec does not allow it).

@andig
Copy link
Member

andig commented Sep 23, 2023

Did you try? An unknown txn id is 0 which is the zero value for integers. Due to the omitempty json tag on the field, Go will not include this in the json.

@pjjjv
Copy link
Contributor Author

pjjjv commented Sep 24, 2023

I will give it a try...

@pjjjv
Copy link
Contributor Author

pjjjv commented Sep 24, 2023

Having trouble to test this. I am getting other errors with 0.120.2 , which I can´t really pin down to a specific cause.

@andig
Copy link
Member

andig commented Sep 24, 2023

Please start discussion with logfile. Be aware of #10029. Lets close here- this has already been confirmed working.

@pjjjv
Copy link
Contributor Author

pjjjv commented Oct 2, 2023

This is working for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants