MyLab-odyssey / ED_BMSdiag

Retrieve battery diagnostic data from your smart electric drive EV.
MIT License
50 stars 19 forks source link

Support for EEPROM config saving and auto-dump of data upon connection #15

Closed jim-sokoloff closed 6 years ago

jim-sokoloff commented 6 years ago

Hi, I created a PR with support for the following features (working):

  1. Support for automatically dumping all data to the serial port upon initial connection to the CAN bus. This is to save the occassional user from having to remember what to type; it becomes plug USB, open serial port, plug CAN, auto-download, done.
  2. Saves user configuration (whether logging is enabled, what frequency, whether initial auto-dump is configured) to the AVR EEPROM so that the next connection comes up in the same state as the device was left in. (If you've done "log on 10", upon next connection, "log on 10" is automatically activated.)
  3. Bumped the master version to 1.0.2.

And preliminary work towards supporting fetching the Car VIN (as distinct from battery VIN), but this is not yet working [though causes no negative effects].

MyLab-odyssey commented 6 years ago

Hi Jim! THX for this nice contribution. Regarding the VIN extraction please just edit the test in the main.ino:

  //test valid VIN stored in battery
  if (sizeof(myVIN) > 1) test_BattVIN();

as it is also displayed by the info command in the CLI.ino:

Serial.print(F("Battery VIN: ")); Serial.println(BMS.BattVIN);

To get a VIN (that is stored in the BMS). The car will issue a VIN within the first minute of activation at IDs 0x560 and 0x6FA when some ECUs will query it.

Can you just revise the code, before I will do a merge? And please keep in mind that I can't test the revision, because I don't own a 451 ED any more.

jim-sokoloff commented 6 years ago

No problem with updating the test, but I'm not exactly sure what you're asking me to do.

I intentionally changed the code to always call test_BattVIN() rather than having it conditionally depend on whether the myVIN define was set, as the code always had myVIN defined (and therefore the if statement was basically "if (true) test_BattVIN()" to set the "HAL>" command prompt.

I was trying to add car VIN support (as distinct from battery) to cover anyone who had a battery replaced [and to teach myself a bit about CAN diagnostics]. I learned that I really needed a better CAN logger (ordered a PiCAN2) to troubleshoot what I was trying to do.

I tested 1.0.2 code pretty thoroughly on my dad's two 451 EDs, so no significant worries there.

If you want a specific code change in the PR, please advise exactly what you want me to do and I'll be more than happy to do it.

Thanks for the consideration of the PR (and the rapid response), --Jim

MyLab-odyssey commented 6 years ago

Okay, I now understand the purpose for your car VIN intention. Go on as intended and watch out for the IDs I mentioned. I will dig into my old logs and show you a readout sample, you can work with. When I have some more time during the holidays I will try to implement the CAN matrix as promised in my milestones.

I am really glad to see someone (you) working with my code. You really analyzed it well!

Best Regards, Ralph

MyLab-odyssey commented 6 years ago

In addition: set the filters for the IDs and you can speed up the readout. See my functions that capture live traffic. I did all the significant scanning with a RPI and a PICAN shield. CAN utils with socket can works very well.

jim-sokoloff commented 6 years ago

Cool. I've got a PiCAN2 on the way and hope to be able to figure out how to adapt this to track the battery info on my LEAF as well as do some testing on a 453 ED to see if this code works as-is or can be made to work with small changes.

Thanks for your original work and let me know if you need anything else from me prior to merging this PR. --Jim

MyLab-odyssey commented 6 years ago

Hi Jim,

as mentioned, please use the ID 0x6FA to get the carVIN. I checked my old logs and this ID is present all time. Here is an example (first data-byte is the index of the message):

6FA, 8, 1 57 4D 45 34 35 31 33 
6FA, 8, 2 39 30 31 4B 37 34 XX 
6FA, 8, 3 XX XX XX FF FF FF FF

(ID 0x560 is only active within a minute or so, with the same format).

Set a filter to 0x6FA and scan the bus for it like I do e.g. with the SOC value. Data is ASCII format, as in the battVIN.

Please revert the edits on the canDiag::PrintReadBuffer function, as this should only output the raw data read by any query. Please don't do any parameter fetch in there.

P.S.: Have you received my PM via smartcarofamerica.com ?

Regards, Ralph

jim-sokoloff commented 6 years ago

Hi Ralph, I did get your PM on smart forum. Unfortunately, I only saw your comment on the PR just now as my github mail was getting filed away by my mail client.

The changes to canDiag::PrintReadBuffer were not doing any CAN or other global operations. They were to print human-readable ASCII format of the raw data where that was available. (That str variable is a local variable, only used in that function.)

Here's a sample: From: Data: 07 41 42 43 44 64

To: Data: 07 41 42 43 44 64 .ABCDd

Adding the translation to characters (or '.' if the value is not printable) so that things like the VIN or other text figures are easier to see in the debug output stream.

I have reverted the edits to that function as you asked, but now that I've explained their purpose, if you want to include them, you can grab them from the 8ee5eae7f8e808b5d78d838429e5d38cc0c84590 commit.

For the car VIN, unfortunately, I'm 630 miles away from where my Dad lives with his 451 ED.

MyLab-odyssey commented 6 years ago

Hi Jim,

just opened a new branch for you PR, as the development seems not finished. Memory is getting tight, but you may manage to include the carVIN feature. Please count up to (at least) v1.1.0 for further PR's, as you added significant new features for a minor release count. The PR will be first merged to the new branch. If everything is working I will do the new release.

THX for your efforts and best regards, Ralph