Fix isMoving sensor

[oliverrahner]

I might be wrong, but doesn't that just do the same as we did it before my last change?

The problem is that we can't/shouldn't call parkingposition regularly. So yes, if it returns 204 it means that the car is moving, but because we don't get a trigger to update the parkingposition when anything changes, that doesn't help us...

Before, this line just put the results from parking position into the states, empty if nothing was returned. Then this line checked for an empty value and deducted if the vehicle is moving or not. You just kind of shifted the logic to another point, but it's still the same.

[stickpin]

@oliverrahner don't you think it's more accurate to rely on the actual HTTP code status? Instead of the response? In any case, I think we should keep the sensor with one implementation or the other.

[oliverrahner]

Now I think I know where the confusion comes from. I noticed that one of the changes I intended to do disappeared... Probably somewhere in my stash. I removed self.get_parkingposition() from the update() call and put it inside get_trip_last(), when a change was recognized there. And then this whole vehicle_moving stuff wouldn't make sense anymore.

[stickpin]

@oliverrahner any outcome from your rate-limit test? For me, IsMoving works fine based on the 204 logic.

Don't you think we should keep this sensor if the rate limit is not triggered?

[oliverrahner]

I agree to your whole approach now, given that we don't reach rate limits anymore. But what concerns me a bit: All last_updated functions (should) also return datetime. But maybe they don't, because we're also just returning strings there. It just kind of assumed that is "implicitly" converted them to datetime, because that's what the original version of the lib also seemed to be doing. Maybe we need to rework all of them to return actual datetimes? And then any necessary conversion for presentation should be done somewhere else.

[stickpin]

I agree to your whole approach now, given that we don't reach rate limits anymore. But what concerns me a bit: All last_updated functions (should) also return datetime. But maybe they don't, because we're also just returning strings there. It just kind of assumed that is "implicitly" converted them to datetime, because that's what the original version of the lib also seemed to be doing. Maybe we need to rework all of them to return actual datetimes? And then any necessary conversion for presentation should be done somewhere else.

Yes, it's also still producing an error once a vehicle is moving:

2023-12-06 08:08:05.665 ERROR (MainThread) [homeassistant] Error doing job: Task exception was never retrieved
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/update_coordinator.py", line 233, in _handle_refresh_interval
    await self._async_refresh(log_failures=True, scheduled=True)
  File "/usr/src/homeassistant/homeassistant/helpers/update_coordinator.py", line 389, in _async_refresh
    self.async_update_listeners()
  File "/usr/src/homeassistant/homeassistant/helpers/update_coordinator.py", line 172, in async_update_listeners
    update_callback()
  File "/config/custom_components/volkswagencarnet/__init__.py", line 318, in async_write_ha_state
    backend_refresh_time = self.instrument.last_refresh
                           ^^^^^^^^^^^^^^^
  File "/config/custom_components/volkswagencarnet/__init__.py", line 381, in instrument
    return self.data.instrument(self.vin, self.component, self.attribute)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/custom_components/volkswagencarnet/__init__.py", line 271, in instrument
    raise ValueError(f"Instrument not found; component: {component}, attribute: {attr}")
ValueError: Instrument not found; component: sensor, attribute: parking_time

[oliverrahner]

I can't test with my own car currently, they broke online connectivity last week when I went for inspection 😬 Hope to have it fixed on Friday. Time to look into some API mocking to test this 😅

[stickpin]

I am waiting for my wife to drive somewhere to get API responses in the so-called "InMoving" state. :)) So it will be easier to debug. I will push it to the repo once I will collect it with the moving state.

[stickpin]

@oliverrahner check my last commit https://github.com/robinostlund/volkswagencarnet/pull/222/commits/cfc0f3a913933660b0d599d7f81823d95957471b this one fixes the issue with parking_time I've mentioned before and I've also optimized the code a bit because now everything is based on the position.