search

noisymime / speeduino

Speeduino - Arduino based engine management
http://speeduino.com
GNU General Public License v2.0
1.31k stars 526 forks source link

Adding new config items and CRC errors #222

Closed ric355 closed 5 years ago

ric355 commented 5 years ago

Is there anything special I need to do to add items to the ini file now that we have CRC support? I have added some items to then end of page=1 (config2 in the code) but whenever I burn them I get a CRC error from TunerStudio.

ric355 commented 5 years ago

It seems like there is an inconsistency between the unused portion in page=1 of the ini file and the definition of the matching config2 structure; they differ by 1 byte;

unused2-67 = array, U08, 71, [56], "%", 1.0, 0.0, 0.0, 255, 0 This starts at the 72nd byte in the structure for 56 bytes, which gives a total size of 127 bytes.

and byte unused1_70[57];

Despite the name, this is actually the 72nd byte for 57 bytes (so it should really by byte unused1_71[57]), which gives a total of 128 bytes.

The size of the page is defined as 128 bytes. So the net result seems to be that TS is calculating a CRC on 127 bytes and the firmware is calculating it on 128 bytes. It seems to be OK for me when I load the base March firmware, but if I extend it and apply the relevant delta to the existing sizes, I start getting CRC errors.

I fixed it by extending the size of the unused structure in the speeduino.ini page = 1 by 1 byte.

My actual sizes are different as I've extended the structure to add other things, but I believe based on the March firmware it should really be: unused2-67 = array, U08, 71, [57], "%", 1.0, 0.0, 0.0, 255, 0

brunob45 commented 5 years ago

I confirm that bug, also happened to me when I added custom settings in page 2 (or 1, depending). With the latest code, which added the ase tables, the size of the unused array should be 36. Also, adding a line like static_assert(sizeof(config2) == 128, "config2 size is not 128"); at the end of globals.h could further help to notice inconsistencies in page sizes, as it would throw an error at compile time if it is wrong.

brunob45 commented 5 years ago

Just created a PR (#224). Please check it out :)

noisymime commented 5 years ago

Closing this one off as it looks to be resolved now. PR #224 was merged, but this did not fix the issues and I eventually found the problem was cause by c7063eb26878ac823fdc3abd08e99322fc1d179f in which I incorrectly changed the scope of one of the variables used in comms.ino

Everything looks to be working again now with the fix in 827552a8fd017fd3bb6cf3b081807a9216aa0723