Serial Communication Take 2

Official FreeEMS vanilla firmware development, the heart and soul of the system!
User avatar
Fred
Moderator
Posts: 15431
Joined: Tue Jan 15, 2008 2:31 pm
Location: Home sweet home!
Contact:

Re: Serial Communication Take 2

Post by Fred »

It's clear that we need to move the protocol spec from the current simple scheme of:
  • FirmwareVersion = "arbitrary string that changes every single time it's built differently"
  • InterfaceVersion ="family / code name string + space + numeric version"
To something more robust and better broken up into distinct tasks:
  • FirmwareName = "arbitrary string that never changes for a single firmware" (for FreeEMS releases from me, it'll always be "FreeEMS Official" or vanilla or pure or certified or similar)
  • FirmwareVersion = "arbitrary string that changes every single time it's built differently"
  • InterfaceVersion ="numeric version of form N.N.N etc with at least one N and number of dots = number of Ns minus one"
Or possibly even this:
  • FirmwareName = "arbitrary string that never changes for a single firmware" (for FreeEMS releases from me, it'll always be "FreeEMS Official" or vanilla or pure or certified or similar)
  • FirmwareVersion = "arbitrary string that changes every single time it's built differently" (this lets us track bugs to specific code versions and is expected to change often without necessarily changing the config of api versions)
  • CommAPIVersion = "numeric version same as above but related to the serial functions and their formats" (this tells us how data and functionality access in the device is implemented and when changed requires a code change in the tuning software to match it
  • ConfigFormatVersion ="numeric version of form N.N.N etc with at least one N and number of dots = number of Ns minus one" (this tells us about what data is available and in which locations which functions are.)
From this last scheme we can do a few nice things such as:
  1. Check the FirmwareName and if unknown, refuse to function, if known, but not supported, say so, or if supported, continue.
  2. Given a known and supported FirmwareName:
  3. Check the CommAPIVersion and if unknown, refuse to function, if known, but not supported, say so, or if supported, continue.
  4. Given a known and supported CommAPIVersion:
  5. Check the ConfigFormatVersion and if we give the different parts special meaning, proceed with caution, proceed advised against, or refuse to proceed on the basis of likely corruption.
The same could be done for the CommAPIVersion. For example, we could say that patch/incremental/micro version increases are backward compatible and only add functionality or remove functionality, but don't change any existing IDs from other versions with the same major/minor versions. Likewise we could say that minor version number changes break compatibility, but should be easy to adjust for support to, not fundamental major changes, and major version number changes, except for from 0 to 1 are considered "anything goes" changes.

For the config format version we could do the same thing, ie, micro/patch/incremental version changes mean addition of new data in a previously unused place and/or removal of usage of data such that bogus data in those locations does not affect existing functionality as it would if moved there. Thus micro version changes could be supported by external tools with no change. Minor version changes denote non backward compatible changes that require redoing the mappings from data to functionality to avoid corruption of configuration possibly leading to engine damage.

Note, we'll need these anyway because it's unreasonable to assume that a tuner will definitely be able to pull config from the device and indeed to assume that the firmware will have the config in it, imagine cut down versions running on smaller less capable chips such as my incomplete FreeMS2 fork.

The name is simple, either you know about it or not, and support some of it or not. Easy.

The firmware version is just an identifier, it can't and shouldn't be used to distinguish anything of interest except for which firmware has which bug in it.

Fred.
DIYEFI.org - where Open Source means Open Source, and Free means Freedom
FreeEMS.org - the open source engine management system
FreeEMS dev diary and its comments thread and my turbo truck!
n00bs, do NOT PM or email tech questions! Use the forum!
The ever growing list of FreeEMS success stories!
dandruczyk
LQFP112 - Up with the play
Posts: 100
Joined: Sun Apr 06, 2008 6:30 pm

Re: Serial Communication Take 2

Post by dandruczyk »

FirmwareVersion/Interface Version should be 3 or 4 element structures of unsigned integers NOT a string..
Strings are ugly to parse, and zero's can be misinterpreted as nul's.

a simple:

Code: Select all

FWVersionStruct
{
  uint8 major;
  uint8 minor;
  uint8 micro;
};
If you're worried about possible hitting over 256 major/minor or micro's use 16 bit fields instead. These would be read from flash so RAM isn't much of a concern anyways for this set of calls.
User avatar
Fred
Moderator
Posts: 15431
Joined: Tue Jan 15, 2008 2:31 pm
Location: Home sweet home!
Contact:

Re: Serial Communication Take 2

Post by Fred »

dandruczyk wrote:FirmwareVersion/Interface Version should be 3 or 4 element structures of unsigned integers NOT a string...
Interface version should be numeric with separators of some type, yes, BUT, firmware version should definitely NOT be numeric, no way, it's purely an ID to be pulled out and displayed and/or recorded and the format is likely stay more or less as is, ie, a variable and custom string. For example, right now, this "0.2.0-SNAPSHOT-8-g2e9dd95-DEV"
Strings are ugly to parse, and zero's can be misinterpreted as nul's.
No they can't, zero is 0x30 in ASCII and ISO-8859-1/Latin1 :-p null/0x00/0 value = end of string, that's universal. If one ends up in the middle of a string, it's because someone is stupid, if it ends up in FreeEMS official, that person was me and you should send suitable abuse/issues on trackers/etc :-)
a simple:

Code: Select all

FWVersionStruct
{
  uint8 major;
  uint8 minor;
  uint8 micro;
};
If you're worried about possible hitting over 256 major/minor or micro's use 16 bit fields instead. These would be read from flash so RAM isn't much of a concern anyways for this set of calls.
I'll consider numbers, but yes, they'd have to be 16 bit as there will easily be 256+ changes to micro/patch/incremental before things are complete and stable, no question. Also, I don't like the fixed 3 part format. What happens when:
  • You're at 1.2.45 and it's released and people are using it.
  • You have started developing and working on 1.2.46-SNAPSHOT with 30 commits in there
  • Someone comes along and finds a bug in 1.2.45 which should be fixed and released immediately
You could release 1.2.46 from the bug fix branch and migrate to 1.2.47-SNAPSHOT dev versions, but that is dirty and wrong...

The correct thing to do is to release 1.2.45.1 with the fix included and NOTHING else. and then integrate the fix into 1.2.46-SNAPSHOT code such that it's also included in 1.2.46.

Thus if we go numeric, then it would have to be an arbitrary length array where payload / 2 = number of parts in the version.

I still don't like it much, though, as it's kind of hard coding the versions to be numeric, and rules out alphanumeric versions and other formats that someone, possibly me, down the line may want to use for some valid and currently un-thought-of reason.

Fred.
DIYEFI.org - where Open Source means Open Source, and Free means Freedom
FreeEMS.org - the open source engine management system
FreeEMS dev diary and its comments thread and my turbo truck!
n00bs, do NOT PM or email tech questions! Use the forum!
The ever growing list of FreeEMS success stories!
dandruczyk
LQFP112 - Up with the play
Posts: 100
Joined: Sun Apr 06, 2008 6:30 pm

Re: Serial Communication Take 2

Post by dandruczyk »

Fred wrote:
dandruczyk wrote:FirmwareVersion/Interface Version should be 3 or 4 element structures of unsigned integers NOT a string...
Interface version should be numeric with separators of some type, yes, BUT, firmware version should definitely NOT be numeric, no way, it's purely an ID to be pulled out and displayed and/or recorded and the format is likely stay more or less as is, ie, a variable and custom string. For example, right now, this "0.2.0-SNAPSHOT-8-g2e9dd95-DEV"
Strings are ugly to parse, and zero's can be misinterpreted as nul's.
No they can't, zero is 0x30 in ASCII and ISO-8859-1/Latin1 :-p null/0x00/0 value = end of string, that's universal. If one ends up in the middle of a string, it's because someone is stupid, if it ends up in FreeEMS official, that person was me and you should send suitable abuse/issues on trackers/etc :-)
a simple:

Code: Select all

FWVersionStruct
{
  uint8 major;
  uint8 minor;
  uint8 micro;
};
If you're worried about possible hitting over 256 major/minor or micro's use 16 bit fields instead. These would be read from flash so RAM isn't much of a concern anyways for this set of calls.
I'll consider numbers, but yes, they'd have to be 16 bit as there will easily be 256+ changes to micro/patch/incremental before things are complete and stable, no question. Also, I don't like the fixed 3 part format. What happens when:
  • You're at 1.2.45 and it's released and people are using it.
  • You have started developing and working on 1.2.46-SNAPSHOT with 30 commits in there
  • Someone comes along and finds a bug in 1.2.45 which should be fixed and released immediately
You could release 1.2.46 from the bug fix branch and migrate to 1.2.47-SNAPSHOT dev versions, but that is dirty and wrong...

The correct thing to do is to release 1.2.45.1 with the fix included and NOTHING else. and then integrate the fix into 1.2.46-SNAPSHOT code such that it's also included in 1.2.46.

Thus if we go numeric, then it would have to be an arbitrary length array where payload / 2 = number of parts in the version.

I still don't like it much, though, as it's kind of hard coding the versions to be numeric, and rules out alphanumeric versions and other formats that someone, possibly me, down the line may want to use for some valid and currently un-thought-of reason.

Fred.
I meant packed structures of NUMBERS for interface and comm API versions, not for the firmware name and family name as erroneously stated above.

In your previous instance where you have uint8,uint8,uint8,string, I had to be VERY careful to get offsets right as string functions would see the zero in the first three fields and assume it was the string terminator. hence I'm not a big fan of mashing together integers and strings together, unless the structure is set in stone and tightly defined (though I think separate calls for numerical info vs string info is best)
User avatar
Fred
Moderator
Posts: 15431
Joined: Tue Jan 15, 2008 2:31 pm
Location: Home sweet home!
Contact:

Re: Serial Communication Take 2

Post by Fred »

In your previous instance where you have uint8,uint8,uint8,string, I had to be VERY careful to get offsets right as string functions would see the zero in the first three fields and assume it was the string terminator. hence I'm not a big fan of mashing together integers and strings together, unless the structure is set in stone and tightly defined (though I think separate calls for numerical info vs string info is best)
Yes, the old way was dirty and plain shit, and my fault, and needed to be fixed :-) We'll keep hashing it out here and the shower until a final decision is made, then we'll go forward and try to forget about it. At least some of the code smell has gone now! :-)
DIYEFI.org - where Open Source means Open Source, and Free means Freedom
FreeEMS.org - the open source engine management system
FreeEMS dev diary and its comments thread and my turbo truck!
n00bs, do NOT PM or email tech questions! Use the forum!
The ever growing list of FreeEMS success stories!
User avatar
Spudmn
LQFP112 - Up with the play
Posts: 232
Joined: Thu Feb 10, 2011 12:27 am
Location: Auckland, NZ

Re: Serial Communication Take 2

Post by Spudmn »

Hi All

Is there a document that describes the existing data logging protocol spec or will I need to look at the source code?
User avatar
Fred
Moderator
Posts: 15431
Joined: Tue Jan 15, 2008 2:31 pm
Location: Home sweet home!
Contact:

Re: Serial Communication Take 2

Post by Fred »

Spudmn wrote:data logging protocol spec
What is this? Do you mean the payload contents? Scalers, names, units, etc? If so, refer to OLV source, MTX xml, or freetune C++. If you mean the packet structure in general, then yes, two PDFs cover all comms stuff, excluding tuning data format stuff.
DIYEFI.org - where Open Source means Open Source, and Free means Freedom
FreeEMS.org - the open source engine management system
FreeEMS dev diary and its comments thread and my turbo truck!
n00bs, do NOT PM or email tech questions! Use the forum!
The ever growing list of FreeEMS success stories!
User avatar
Spudmn
LQFP112 - Up with the play
Posts: 232
Joined: Thu Feb 10, 2011 12:27 am
Location: Auckland, NZ

Re: Serial Communication Take 2

Post by Spudmn »

If you mean the packet structure in general, then yes, two PDFs cover all comms stuff, excluding tuning data format stuff.
Yes that's the ones.
I found the latex versions in "freeems-vanilla\docs\migrate\latex"
I have not used latex before (no jokes please) so I looked for the PDF's on the net and found them.

Thanks.
User avatar
Fred
Moderator
Posts: 15431
Joined: Tue Jan 15, 2008 2:31 pm
Location: Home sweet home!
Contact:

Re: Serial Communication Take 2

Post by Fred »

I didn't use latex for more than 10 years, but when I did, I had some unfortunate reliability problems. Nevertheless, the output from latex looks fantastic from any perspective.
DIYEFI.org - where Open Source means Open Source, and Free means Freedom
FreeEMS.org - the open source engine management system
FreeEMS dev diary and its comments thread and my turbo truck!
n00bs, do NOT PM or email tech questions! Use the forum!
The ever growing list of FreeEMS success stories!
Post Reply