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 API support #67

Merged
merged 7 commits into from
May 4, 2020
Merged

Tesla API support #67

merged 7 commits into from
May 4, 2020

Conversation

lincomatic
Copy link
Member

Copy link
Collaborator

@jeremypoulter jeremypoulter left a comment

Choose a reason for hiding this comment

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

A few comments,

for (int i=0;i < _vehicleCnt;i++) {
JsonObject responsei = jresponse[i];
_vin[i] = responsei["vin"].as<String>();
// doesn't work.. returns converted float _id[i] = responsei["id"].as<String>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about:

Suggested change
// doesn't work.. returns converted float _id[i] = responsei["id"].as<String>();
_id[i] = String(responsei["id"].as<int>());

but equally, if the JSON is a number why save into a String? just use int will save memory and be faster

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a 64-bit integer, which is probably why ArduinoMongoose barfs on it. I tried converting my stored string to an integer using strtoul(), and it also barfs:

ID: 28140064781417093 4294967295
ID: 11068998744631046 4294967295

Yes, it's a bit of a waste of memory, but not much

Copy link
Collaborator

Choose a reason for hiding this comment

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

strtoul is for 32-bit, did you try strtoull? or casting to uint64_t? but in either case yeah not much more memory

Comment on lines +435 to +447
if (!_isBusy()) {
if ((millis()-_lastRequestStart) > TESLA_REQ_INTERVAL) {
if (_accessToken.length() == 0) {
requestAccessToken();
}
else if (_vehicleCnt == 0) {
requestVehicles();
}
else {
requestChargeState();
}
}
} // !busy
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure this is the best way of doing this, should really send the next request as soon as the previous one is complete, how about only putting the timer around getting the charge state:

Suggested change
if (!_isBusy()) {
if ((millis()-_lastRequestStart) > TESLA_REQ_INTERVAL) {
if (_accessToken.length() == 0) {
requestAccessToken();
}
else if (_vehicleCnt == 0) {
requestVehicles();
}
else {
requestChargeState();
}
}
} // !busy
if (!_isBusy()) {
if (_accessToken.length() == 0) {
requestAccessToken();
}
else if (_vehicleCnt == 0) {
requestVehicles();
}
else if ((millis()-_lastRequestStart) > TESLA_REQ_INTERVAL)
{
requestChargeState();
}
} // !busy

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem with your suggested fix is that when the credentials are entered incorrectly, or the Tesla servers are just having issues, we're going to be hitting up their servers at a very high rate. Do you really want to do that? If so, I'll commit your suggestion

Copy link
Collaborator

Choose a reason for hiding this comment

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

True, should probably back off a little under error conditions, Ideally these should be feed back to the UI, but we are not very good at that anywhere :(

@glynhudson
Copy link
Collaborator

This a lot @lincomatic

From @chris1howell, here are they API web calls for this integrations

Telsa user/password/vehicle_index/enable are saved to EEPROM
 
web server interface stuff:
 
http://openevse/savetesla?user=TESLAUSER&pass&TESLAPASS&enable=true

set enable to false to disable it, just like the existing stuff
 
http://openevse/saveteslavi?vi=VEHICLEINDEX

to select which vehicle to fetch charge data for
 
`http://openevse/teslaveh

returns JSON of vehicle info e.g

{"count:"2,[{"id":"xxxx","name":"tesla1"},{"id":"xxxxx","name":"tesla2"}]}
 
http://openevse/config

has tesla_user:TESLAUSER,tesla_vehidx:VEHICLEIDX added to the JSON

tesla_user":"[email protected]","tesla_vehidx":1,

 
http://openevse/status

has tesla charge info added to the JSON

"batteryRange":122.49,"chargeEnergyAdded":24.90,"chargeMilesAddedRated":84.50,"batteryLevel":51,"chargeLimitSOC":86,"timeToFullCharge":0,"chargerVoltage":0,

Data is also published to mqtt:

unnamed

@jeremypoulter
Copy link
Collaborator

One more thing occurred to me, we should probably split the MQTT topics in to generic car charge state and Tesla specific stuff. @glynhudson What is the comparable info that OBMS can provide?

Comment on lines +1196 to +1197
server.on("/savetesla$", handleSaveTesla);
server.on("/saveteslavi$", handleSaveTeslaVI);
Copy link
Collaborator

Choose a reason for hiding this comment

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

As an FYI I am currently adding some code that will make these redundant, all new config will be saved by a POST to /config but I will update when I merge. This change means you just have to add two lines in `app_config.cpp' and that will the new config option.

Copy link
Member Author

Choose a reason for hiding this comment

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

excellent

Copy link
Collaborator

Choose a reason for hiding this comment

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

Take a look at #76, will probably merge this PR first so I can resolve the conflicts

@lincomatic
Copy link
Member Author

Sorry, but I'm pretty swamped these days, and I won't have time to do anything more on this for a while. You're welcome to change it to suit your tastes if there's anything outstanding that you don't like about my implementation. Thanks.

@jeremypoulter jeremypoulter mentioned this pull request May 3, 2020
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