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

[tesla] Add channel for raw JSON response from the upstream API #15889

Closed

Conversation

hakan42
Copy link
Contributor

@hakan42 hakan42 commented Nov 13, 2023

Inspired by #15580 and discussions I had earlier about creating channels for little-used properties, this enhancement creates an advanced channel to expose the RAW JSON response of the upstream API for the "vehicleData" calls.

That way, an user will be able to access any data points in the API via the JSONPATH transformation.

@hakan42 hakan42 added the enhancement An enhancement or new feature for an existing add-on label Nov 13, 2023
@hakan42 hakan42 requested a review from kgoderis as a code owner November 13, 2023 13:55
@hakan42
Copy link
Contributor Author

hakan42 commented Nov 14, 2023

@lsiepel , may I again draw your attention to another pull request ?

@lsiepel
Copy link
Contributor

lsiepel commented Nov 14, 2023

@lsiepel , may I again draw your attention to another pull request ?

While it may fit in some usecases, my personal opinion remains the same, i don't like raw channels. I prefer to model the API to the openHAB domain to keep it inline with the openHAB architecture. But the other PR also got merged, so it seems fine from maintainers perspective.

@lolodomo
Copy link
Contributor

lolodomo commented Nov 14, 2023

No the other PR was not yet merged.
And I tend to agree with you @lsiepel
This should be discussed with OH leaders before creating new PR of this kind.

@lolodomo
Copy link
Contributor

@openhab/add-ons-maintainers : WDYT about such proposal ?

@kaikreuzer
Copy link
Member

I also agree with @lsiepel here. Unless there is really a strong reason for it, we should avoid raw channels.
@hakan42 What information do you see in the JSON that is not reflected in the existing channels? Imho, the Tesla API does not change that often and we should rather make sure to provide "semantic" channels for any new additions.

@hakan42
Copy link
Contributor Author

hakan42 commented Dec 10, 2023

I would argue that the current owners API is reverse engineered but not really something defined by Tesla. And therefore prone to change. The other part is, it feels like ... missing a proper english word for "Fleißarbeit" here , maybe grind? :)

I might be inclined to define "semantic" channels for all of the old json results, but for this the internals of the mapping of json trees to channels would need a bit of rework.

In the end, it would depend on how long the owners' API does live on, perhaps wait until February or so to see where Tesla goes with the fleet API and whether they let the current one alive? WDYT @kaikreuzer ?

@kaikreuzer
Copy link
Member

If tesla removes or heavily changes the owner's API, the binding anyhow would require a rework - and having the raw channel wouldn't help here, if the API does not exist anymore. But in the past years, not much has happened and we only added a few new channels over time.
I don't see anything missing in the current set of channels and hence I do not see the need for any change right now.

@hakan42
Copy link
Contributor Author

hakan42 commented Dec 10, 2023

O.k., let me withdraw this pull request then, and create fresh ones for specific channels when I identify a proper need for them.

Thank you all for your insights and comments :)

@lolodomo
Copy link
Contributor

I close it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants