custom-components / zaptec

zaptec charger custom component for home assistant
MIT License
67 stars 20 forks source link

Tested and verified #35

Closed gustafssone closed 1 year ago

gustafssone commented 1 year ago

A few points here. Yes, it is working for three phase after a few changes. Added working services.py - "service_handle_limit_amps". Also removed the service call for single phase. However it is "requesting" all three amps, meaning single phase not working here. Not sure how to go about that. Sounds good with a single service call but can it dynamically handle both single and 3 phase service call? api.py: header["Accept"] = "Application/patch-json+json" - makes it NOT work await resp.json(content_type=None) - also seems to be required.

gustafssone commented 1 year ago

@elden1337 single service call not working yet. @Hellowlol the schema in services.py is the cause of the issue, not dynamically handling all three phases. It is strict (request) about all 3 phases being input. My initial suggestion was to have two different service call so not sure how to make this dynamic. You also mentioned "check the path" .. Should it check that it is only using url path "installation" or what do you mean?

gustafssone commented 1 year ago

@Hellowlol how can I assist to help this move forward?

Hellowlol commented 1 year ago

? Have you checked the services that is available with the beta version?

gustafssone commented 1 year ago

Which services?

gustafssone commented 1 year ago

Are you referring to the service limit_amps? It has been connected to the service_call "service_handle_limit_amps", but not sure how to dynamically switch between 1 and 3 phase.

Hellowlol commented 1 year ago

The check services.py file

gustafssone commented 1 year ago

Oh, well I took that version v0.0.6b2 and modified it further, so it was the latest one I modified.

gustafssone commented 1 year ago

@Hellowlol I feel this got stalled a bit. Not sure how to deal with a dynamic 1/3 phase service. How can that be handled programmatically?

sveinse commented 1 year ago

@gustafssone I like this fix! I was doing something similar in my own branch, and then I found this PR. I've made some adjustments to your PR (https://github.com/sveinse/zaptec/commit/55800d5d6a9d157071a93d6bf5db437106d3f3b9): Renamed the service to limit_current and made support for setting either one/all phase setting or individual 3-phase setting. This is tested on 3-phase Zaptec Go HW. https://github.com/sveinse/zaptec/tree/feature-gustafsone

I hope this can be useful. And I hope this can be included into the main repo soon.

gustafssone commented 1 year ago

Seems to work! Great, now I don't have to figure out the code 👍 Not sure if this is needed, but if I accidentally send the following: service: zaptec.limit_current data: installation_id: 6dxx available_current: 13 available_current_phase1: 15

I get: Failed to call service zaptec.limit_current. extra keys not allowed @ data['available_current_phase1'], extra keys not allowed @ data['available_current']. Got {'type': 'execute_script', 'sequence': [{'service': 'zaptec.limit_current', 'data': {'installation_id': '6dxx', 'available_current': 13, 'available_current_phase1': 15}}], 'id': 45} Should it be handled in any other way? Foolproof the code?

gustafssone commented 1 year ago

Same thing with the standard YAML data, all 4 types (available_current, available_current_phase1, available_current_phase2, available_current_phase3) causes same type of error.

sveinse commented 1 year ago

@gustafssone I updated my branch with an additional message specifying that available_current and available_current_phaseX cannot be intermixed. Do you think this message is clearer?

sveinse commented 1 year ago

@gustafssone I took the liberty to update (and improve) the fix in this PR in PR #48 for the newest master. I hope that was ok.

If PR #48 is implemented, this PR can be closed.

Hellowlol commented 1 year ago

@gustafssone Thanks for this PR. Nice work. I will close this PR as it has been added in the latest beta