Skip to content

fix/feat:Add support for One Off Charging Timeres as well as start_climatisation fix - #653

Open
max-2468 wants to merge 7 commits into
skodaconnect:mainfrom
max-2468:charging_timers_fix
Open

fix/feat:Add support for One Off Charging Timeres as well as start_climatisation fix#653
max-2468 wants to merge 7 commits into
skodaconnect:mainfrom
max-2468:charging_timers_fix

Conversation

@max-2468

Copy link
Copy Markdown
Contributor

While doing some tests regarding PR #640 I did stumble across the toggle for start the climatisation of the car as well. Switching that on made a new field appear in the data send by the server.

Apparently the app/server/car will only notice this field once it has been manually set once, and will default a missing value to False. To handle this new field while ensuring compatiblity to the behaviour shown by the app (and not having to manually edit a lot of fixtures as well as ensuring compatibility for new fixtures) a special type was used to handle the distinction True, False, Unknown/None.

The handling of One_off charging timers is based on PR #640 by ethanhawke-gif.

This PR includes:

  • new fixtures testing all options available on charging profiles/charging timers, one_off timers as well as the climatisation switch
  • an updated handling of fixture test cases to handle the optionality of new fields (by doing a complete JSON => model => JSON roundtrip and then loading both the resulting JSON as well as the fixture as a dict).
  • some more fixes to the charging profile test case

@dvx76 dvx76 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!

Can you please look at replacing the tribool package?

For the one_off_day fix, either remove that from your PR, or rebase on top of #640 . Don't create your own commit with the same change.

)
from mashumaro.mixins.orjson import DataClassORJSONMixin
from mashumaro.types import SerializationStrategy
from tribool import Tribool

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try to implement this without the tribool package.

This adds another package dependency for something relatively trivial. More importantly that package hasn't been updated since 2016 and includes some questionable behavior (e.g. bool(Tribool(True)) raises a TypeError)

The start_climatisation field can just be typed as bool | None.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants