-
Notifications
You must be signed in to change notification settings - Fork 106
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
open to a possible API refactor? #107
Comments
Open to this. It's due for refactoring. If backward compatibility is broken I would want to take the opportunity to add better error handling that is more like request() and to unbundled all the examples that have taken on a life of their own. |
@mreinstein A small note to say that the TeslaJS library does something similar for token handling. |
Agreed on removing JSONbig dependency. This was added long before tesla added the id_s field so it's time for it to go. |
Finally finished the token handling enhancement request. Not necessary to pass it into every call. I created at set_token() function that sets it once and uses it for the remainder of the session. Also added "token" as an optional parameter where ever username and password was used before. Calling any function which used to take username/email and password with a token will now set the token for use in all other functions just as they were before. On the JSONbig dependancy I still would need it or "id" and "id_s" would not resolve to the same values no? |
what I'd like to change:
getToken(email, password, callbackFn)
get_charge_state( vid, token, cb )
JSONBig
module dependency. Rather than fixing the id field, use theid_s
field, which tesla already includes for vehicles. One less dependency.if any/all of these changes would be welcome, I'd be happy to send some PRs.
One thing to note, this would be a significant API change. We'd have to mark this as 2.x to respect semver.
@hjespers thoughts?
The text was updated successfully, but these errors were encountered: