Skip to content
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

Add forbidden capabilities #167

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

WebSpider
Copy link
Contributor

Forbidden capabilities are capabilities that cannot be present for a sensor to be added.
They are effectively the reverse of required capabilities, that must be present for a sensor to be added.

The logic added in this PR is follows:

  • A sensor will be added when the vehicle has none of the capabilities in forbidden capabilities and all of the capabilities in required capabilities

Example here is the battery-care switch for a Superb iV:
The Superb iV does not have the function, but it does support the CHARGING capability.

When you add the CHARGING_MQB capability to the forbidden capabilities, the battery-care switch will now never be added for the Superb iV:

@WebSpider WebSpider requested a review from dvx76 November 3, 2024 20:47
@WebSpider
Copy link
Contributor Author

This is related to #166

Copy link
Member

@dvx76 dvx76 left a comment

Choose a reason for hiding this comment

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

While it looks like this will be usable for the specific case of #166 , I do wonder if this isn't oversimplifying things. Wouldn't it be conceivable that some models with the CHARGING_MQB capability support the charging limit, while other don't?

Wouldn't it be better to use the errors in the response from the charging (and I guess others) endpoint, which explicitly conveys the information we need?

E.g. from the suber_iv fixture:

    errors:
    - description: Care mode is not available.
      type: CARE_MODE_IS_NOT_AVAILABLE
    - description: Auto unlock is not available.
      type: AUTO_UNLOCK_IS_NOT_AVAILABLE
    - description: Charge limit is not available.
      type: CHARGE_LIMIT_IS_NOT_AVAILABLE

@WebSpider
Copy link
Contributor Author

I guess without the knowledge of the actual API structure, we can only guess at what would be conceivable and what not. We have to make an educated guess at some point, this was mine ;-)

I would like think Skoda implemented the CAPABILITIES for a reason, but yes, you are right, it is conceivable a car with the CHARGING_MQB capability has other CHARGING related capabilities. When I look at what Wikipedia says on the MQB platform, it does seem unlikely, which is why I chose this path:

https://en.wikipedia.org/wiki/Volkswagen_Group_MQB_platform#MQB_Evo_(2019)

@WebSpider WebSpider merged commit 1ec3cbb into skodaconnect:main Nov 5, 2024
3 checks passed
@WebSpider WebSpider deleted the forbidden-capabilities branch November 5, 2024 11:23
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.

2 participants