feat: India region support #499
Conversation
|
Thanks for doing this! I'll run through feedback this weekend. Are you able to cleanup merge conflicts and potentially rebase? It shows a lot of changes that may not be real. It will help with review. Version commit also needs to be backed out. The system does that. |
india.py
Outdated
There was a problem hiding this comment.
This should either:
- Removed.
- Moved to the test folders and we load your creds into a secret in the repo so these tests are run like EU. No pressure though.
| @@ -45,7 +57,15 @@ class DayTripInfo: | |||
| """Day Trip Info""" | |||
|
|
|||
| yyyymmdd: str = None | |||
| summary: TripInfo = None | |||
There was a problem hiding this comment.
Be removing this it will break other regions. I also think adding the below will break other regions. Can we conform to existing?
| @@ -11,16 +12,27 @@ | |||
| _LOGGER = logging.getLogger(__name__) | |||
|
|
|||
|
|
|||
| @dataclass | |||
There was a problem hiding this comment.
Location is already handled and used in other values. No need for a new class.
| @@ -1,5 +1,6 @@ | |||
| # pylint:disable=missing-class-docstring,missing-function-docstring,wildcard-import,unused-wildcard-import,invalid-name | |||
| """Vehicle class""" | |||
| import decimal | |||
| @dataclass | ||
| class TripInfo: | ||
| """Trip Info""" | ||
|
|
||
| hhmmss: str = None # will not be filled by summary |
There was a problem hiding this comment.
Removing fields will break other regions and not work for kia_uvo. Please try to use the existing fields if possible.
| self._update_vehicle_properties(vehicle, state) | ||
| state = self._get_maintenance_alert(token, vehicle) | ||
| self._update_vehicle_maintenance_alert(vehicle, state) | ||
| state = self._get_location(token, vehicle) |
There was a problem hiding this comment.
This needs protective logic. By doing this in a cached call you will kill the vehicle battery. Cached calls shouldn't hit the car each call.
|
|
||
| def force_refresh_vehicle_state(self, token: Token, vehicle: Vehicle) -> None: | ||
| # FIXME: Gowtham - No force refresh | ||
| _LOGGER.error(f"Disabling force refresh") |
There was a problem hiding this comment.
Add domain like other logging to clarify what is throwing this. I assume this exists since force refresh command doesn't work yet?
|
any updates on this? |
|
Hi @gowthamgowtham any update? |
|
any updates |
|
@gowthamgowtham any update? |
Works for India region. I haven't managed to add new tests. I am sure some cleanup will be necessary.