"isActive" returing an incorrect value.

[ColinRobbins]

Describe the bug
The value of the Readiness parameter isActive is sometimes incorrect.
The WeConnect API returns the isActive paramter in its data set when the vehicle is active, but does not return anything when the vehicle becomes inactive.
Thus, the update function, which updates the local data dictionary does not update the isActive parameter, as the parameter is absent.
Thus the data dictionary incorrectly reports the vehicle as active, when it is not.

The issue probably affects outher parameters as well - isActive is my test case!

To Reproduce
Run the API, with the EV off.
Run the update funtion
Turn EV on.
Run the update function
Turn EV off
Run the update function - the API will show the EV incorrectly as being active.

Expected behavior
When the EV is switched off, the update function should recognise the absence of the parameter indicate the EV is off, and update the data dictionary accordingly

Screenshots
N/A

Logs
N/A

Additional context
Issue reported in downstream Home Asssiant integration here: mitch-dc/volkswagen_we_connect_id#96

[tillsteinbach]

Hmm, when the parameter is absent its enabled attribute should be false. Is that not the case for you?

[ColinRobbins]

No, that does not appear to be the case.

[pplucky]

Maybe this info can help

[ColinRobbins]

I started to try and understand the code here.
It seems about the time @pplucky is reporting the error being introduced the .enabled parameter on an attribute was modified.

I can see that when the vehicle is updated, enabled should be set to false if the attribute is not present in the data returned from the EV.

I don’t fully understand the code yet, but I cannot see where the enabled parameter is checked when accessing the dict value. So I think the old value from the dict is being accessed.

Should library users be checking the enabled status, before accessing the dict value?

[tillsteinbach]

Yes, it is necessary to check the enabled value. As we are speaking I think I might change in the library that an exception is thrown when accessing a disabled value.

[ColinRobbins]

Thanks. Fixed downstream

[ColinRobbins]

I have reopened this, as I think there is a logic error.
The behaviour I am seeing is

• ‘isActive’ is ‘on’. So the value is on and enabled is false.
• IsActive is then omitted indicating off. So enabled is set to false, but the value is still on.
• When isActive changes next time to on, the logic is
  

  WeConnect-python/weconnect/addressable.py

  Line 227 in b629b0e

  if not self.enabled and valueChanged:

  
  however, valueChanged will be false, so the enabled flag will not be reset to True.

Not sure of the best way to fix it.
Should the value be set to ‘off’ when disabled?
Should the valueChanged test be removed?

[tillsteinbach]

Oh wow, I think you are right. The check is wrong. I will doublecheck if that breaks something and otherwise push a fix tomorrow!
Great catch!

[tillsteinbach]

@ColinRobbins can you check 0.50.1?

[ColinRobbins]

Thanks - trying it now. Need two car journeys to see if it has worked!

…

On Wed, Nov 30, 2022 at 6:52 AM Till Steinbach ***@***.***> wrote: @ColinRobbins <https://github.com/ColinRobbins> can you check 0.50.1? — Reply to this email directly, view it on GitHub <#85 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADGXZ6LESOF6JETMKOCKLB3WK32RHANCNFSM6AAAAAASMCIYGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[pplucky]

@ColinRobbins can you check 0.50.1?

Looking good, for the last couple of weeks.

[ColinRobbins]

This seems fixed to me, closing