-
Notifications
You must be signed in to change notification settings - Fork 65
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
feat(EU): New KIA models #480
Conversation
Not everything is implemented, yet. Only the most important attributes from the vehicle status request are integrated. Therefore, at least some of the open issues could be closed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, please allow me a couple of days to test locally as an older car owner
hyundai_kia_connect_api/Vehicle.py
Outdated
@@ -74,6 +74,8 @@ class Vehicle: | |||
year: int = None | |||
VIN: str = None | |||
key: str = None | |||
protocolType: int = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you follow current naming convention?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean instead of Camel Case notation I should use underscore? Good point, I will fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If protocol type isn't used in an if statement maybe we don't record it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed the naming and removed the attribute as proposed above. Additionaly, I continue with the mapping of the attributes.
I can't figure out how to post the review from mobile app. Only thought is maybe renaming the ccs variable in vehicle to support multiple regions? I doubt other regions will call it that but still have versions. Thoughts on making it more universal? |
I think we can still refactor it, if it is the case. For me it looks a bit like a universal attribute but I might be wrong. |
Each region is totally different. Ev9s haven't had issues so far for other regions as an example. |
I did see in older logs that the attribut Ccuccs2protocolsupport is set to 0 and in my sniffed code it is set to 1. So my interpretation was, that ccs2 was disabled in the past and now - in some newer cars - it is enabled. But it is only an interpretation which might be completely wrong. The attribut could also mean Combined Charging System (CCS) but then why would it be included in all calls? |
Interesting. So far I haven't seen this value in any other regions. I have seen a version flag though. Not sure why each region is so different. Stamps also don't exist in ca and USA. They do exist in Australia though. |
Austria is following EU style, right? Similar to IN and CN. |
Yes. |
I head to add the attribute Ccuccs2protocolsupport to the header. The value is taken from the result of the vehicle call. It would be good if somebody could test it for the old cars in EU. |
I have a 2024 Kona and just tried this PR using a small test script, I do see a lot more data now being returned, including a lot of driving statistics, great work! |
Thanks! I mapped some additional attributes and documented the still missing ones. |
0001-Try-to-map-more-values.patch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good with this being merged. @fuatakgun let us know when you test on your car.
All are integrated now. Thanks for your help. |
Some fixes for previous mapping. I also mapped is_locked to driver door lock. Would be nice to get sniffing of the open lock and setting of the limits. |
I added the proposed changes and also changed the is_locked. All door are checked and only if all four are closed then I set the attribute to True. |
Btw there is Lock status that is inverse its different from open. Property is Cabin.Door.Row1.Driver.Lock its 1 when its open and 0 when its locked. |
Thank you for this! I am going to merge and release this. Another PR can be done for the other work that is on the go. This will help test the current progress out in a wider audience. |
I don't have FrontLeftWindow and FrontRightWindow in HA, only the door, for the back I do see the door and window status |
The kona has vehicle to load, in the data field I see the following, looks this is not exported separately to HA
|
See #483 for the window status |
No description provided.