Serial protocol pre-design discussion - give us your input!

[Fred]

Code: Select all

/* Serial interface unique identifier */
const unsigned char interfaceVersionAndType[19] = {0,0,0,'I','F','r','e','e','E','M','S',' ','V','a','n','i','l','l','a'};
/* This should only change when the serial interface changes (even a little) */
/* This field consists of 3 chars for a 3 part version number and a free form string. For any unique string the version
 * number is also unique. In this way tools can easily support a range of versions for a specific unique string ID */

/* Displayable firmware version identifier */
const unsigned char firmwareVersion[33] = {"FreeEMS Vanilla v0.0.13 pre-alpha"};
/* This should change everytime the code is changed at all (even a little) before each release. */

The last part of the comment for the interface isn't worded the best, but the point is clear. Because you MUST have the string for the number to be meaningful, I can't see a good reason for keeping them separate in code. The serial spec will expect them in "char char char string" form anyway so the source is irrelevant to some extent (at least if adhering to a particular serial spec).

Note that the firmware version and interface version are separate already. They really are totally separate things at the end of the day. The fact that there is no serial interface right now illustrates that clearly :-)

Now I have something to attempt to send in my first serial code :-)

Admin.

[Fred]

All edited...

I'm going to release those odt files, 0.1 can stay 0.1, but 0.1 mark 2 can be 0.1a and from there on we'll do something else for versions as decided by Stu. They do need to be incremental though, just like the code.

EDIT : cancel that, I'll change the second one to 0.2 now.

RE EDIT : sourceforge demands no spaces in filenames (I prefer this too) and unique file names (version in file name? I prefer this too).

https://sourceforge.net/project/showfil ... _id=277306

It would be good if there was a traceable record (besides this thread) of what we started with and how we got to the end point whatever it is. Source forge releases exist in perpetuity afaik, so will do a good job of this.

Now, lets add the request interface and request version commands and descriptions and get 0.3 out (or similar). I'll do this if you don't have time Stu, just let me know. I replaced the spaces with underscores (which I don't like) If you have a better way of avoiding spaces, do or suggest it :-)

Admin.

[Fred]

I guess negative acknowledge can be used for this, but I'd like a way to reject any requests to burn or store data that don't have the checksum field. As I said in the first post, I want all burn/store commands to carry a check sum. Either enforced by the protocol, or I guess by the firmware. I don't want to do it by just ignoring them as that will lead to confusion.

Perhaps negative ack can come with a numeric error that we have a table of meanings for?

0 = unknown/generic error
1 = voltage too low
2 = no checksum sent with burn
3 = no checksum sent with store
4 = ?

We may as well make it a flexi/ber number as then we can keep adding more without limit.

Admin.

[Fred]

Another idea is to include all the basic stuff in a fixed order in the lower part of the protocol ID list. If we do use a bit to ID which list 0 = proto list, 1 = firmware specific list then there are plenty of codes to go around and such a bare minimum list could be included in the proto half of the numbers. This would ease compatibility between various firmwares and external devices like gauges significantly. Using a bit flag doubles the number of ID's we can have for the same sized stretchy/fixed int (stating the obvious i know). I just don't like the arbitrary 20 spaces reserved for protocol use. Splitting it off to a free for all for proto and another for firmware seems nicer.

We still need to see the code for stretchy ints to tell if it is sweet or not and what the field sizes are if it is too slow.

As for the "basic vars" list, we have multiple examples of prior art work to look at to see what is always present, and can add our own default stuff to the existing common list.

If everything is sent raw, the gauge device could perhaps have every var it displays configurable with offset, scaling, max, min and label etc to always be compatible with the particular format used with a particular firmware. (doubling max rpm for bikes inside the same 16 bit field or something similar)

I guess the addressing modes etc (for 3rd party can etc devices) should be defined as a section in part 2 of the spec? I'd forgotten so much about this thread I went back and read through most of it.

Summary thread here :

http://www.diyefi.org/forum/viewtopic.php?f=8&t=219

Keep your comments in this thread though in order to keep that one tidy.

Admin.

[FlappySocks]

I'll do this if you don't have time Stu, just let me know.Admin.

Fine by me Fred. Don't worry, your not stepping on my toes. Been busy developing stuff for my day job.

[Fred]

OK Stu, I'm on it.

Who was it that suggested that the header should be checksummed always? What is your plan for that? Looking at it again, I agree that that is a hole in the robustness of it. would you say : startbyte, flags, payloadtype, acknumber, loadlength, headerchecksum ?, payload, checksum, stopbyte.

What are your thoughts on this Stu?

Admin.

[Fred]

OK, I'm about to release v0.3 of the protocol spec. Things that remain to be discussed and clarified/agreed on :

• CRC semantics, header included, separate, or header not checked, pre escaping, post escaping, 8 bits/16 (I vote 8)
• Flow control - hardware, software, ignoring incoming requests when busy - requirements for each etc.
• Payload type bit flag for protocol/firmware use (eliminate the 20 limit at the expense of a header bit)
• Stretchy ints, yes or no, if yes, how, if no, how big for numbers.
• Appendix A payload format details correct/incorrect etc.
• Include extensive "basic" vars in part 1 spec for compatibility with simple devices that can't cope with flash protocols ;-) ?? (would require detailed thinking...)
• Acknowledgment form. I say empty = positive, full = stretchy int with error code of some sort. Thoughts?

And perhaps more.

I've added quite a few things including :

• per byte bit arrangement details
• redid and improved detail on escape scheme
• Added interface and firmware request details and adjusted other payload type details (limited overall length to 256 bytes for each)
• Added section for flow control
• Added section for reliable transmission (even if it's just a guideline)
• Added length of payload description
• Added pin / device addressing section to part 2

The release :

http://sourceforge.net/project/showfile ... _id=601629

The next release can come from anyone. Whoever puts the work in to update it to match what we have agreed in this thread from this point on can send it to me and I'll put it up on sourceforge.

The updated files and above lists should provide a good reference for further discussions :-)

Thanks to all involved so far.

Admin.

[FlappySocks]

Here are my thoughts.

[*]CRC semantics, header included, separate, or header not checked, pre escaping, post escaping, 8 bits/16 (I vote 8)
8-bit vote for me. If comms is unreliable, it will probably be obvious, and therefore 8-bit is good enough.
My gut feeling is that CRC should be pre-escaping, and the header not checked. i.e. CRC the payload only. If the header is corrupt, it's most likely going to mess the whole frame up anyway.


[*]Flow control - hardware, software, ignoring incoming requests when busy - requirements for each etc.
Hardware, hardware hardware, unless there is a very compelling reason not to.


[*]Payload type bit flag for protocol/firmware use (eliminate the 20 limit at the expense of a header bit)
Good idea. I like this.

[*]Stretchy ints, yes or no, if yes, how, if no, how big for numbers.
Part 1, Yes (where already specified). Part 2, sometimes - eg, where Integers have no known future boundary. No point in specifying a stretchy integer for engine temperature for example, if it's going to melt before the temperature value saturates 1 byte of data! If the unthinkable happened, and new engines run at over 255 degrees, then we will just have to issue a new command payload type, with a revised format. (temperature will probably be a floating number anyway, so bad example, but you get the idea)

[*]Appendix A payload format details correct/incorrect etc.
Looks ok. I wonder if we should specify an end of String character. Byte Zero maybe? Otherwise, how do you send multiple variably sized strings in the same payload.


[*]Include extensive "basic" vars in part 1 spec for compatibility with simple devices that can't cope with flash protocols ?? (would require detailed thinking...)
Yeah, maybe. Perhaps instead of the STX byte 0xaa, you start with another byte, which proceeds with a text based interface. Do we need to worry about this now? Seems like work we dont need right now. Leave it for version 2?


[*]Acknowledgment form. I say empty = positive, full = stretchy int with error code of some sort. Thoughts?
Not sure really. It has a lot to do with how much processing capacity is available in the ECU, and the way the firmware is written.



[*]I've added quite a few things including :

[snip]

Good work!

[Fred]

8-bit vote for me. If comms is unreliable, it will probably be obvious, and therefore 8-bit is good enough.
My gut feeling is that CRC should be pre-escaping, and the header not checked. i.e. CRC the payload only. If the header is corrupt, it's most likely going to mess the whole frame up anyway.

8 bit = yes :-) I'd like to put some more effort into analysing that before we make a decision on the rest though. We can look to see what happens in the case of each bit/ set flipping or even write a program to totally analyse it all maybe. It just seems like the semantics of what we do will have a big effect on whether the data can be trusted or not. There most be discussion on this somewhere in one of the similar protocols linked.

Hardware, hardware hardware, unless there is a very compelling reason not to.

Agreed, just have to look into how/if it is possible/what it would take on our setup.

[*]Payload type bit flag for protocol/firmware use (eliminate the 20 limit at the expense of a header bit)
Good idea. I like this.

Deal :-)

if yes, how

Part 1, Yes (where already specified).

I'm still keen to see the "how" part :-) I like the idea, but depending on the overheads involved, it could make sense to just pad the hell out of the fields so that we don't run out. Trying to be objective and not subjective here :-)

Part 2, sometimes - eg, where Integers have no known future boundary. No point in specifying a stretchy integer for engine temperature for example, if it's going to melt before the temperature value saturates 1 byte of data! If the unthinkable happened, and new engines run at over 255 degrees, then we will just have to issue a new command payload type, with a revised format. (temperature will probably be a floating number anyway, so bad example, but you get the idea)

Part 2 should be defined by a particular interface version right? So it seems to not be of any use in that context. The bit count of a field will need to be defined clearly in order for it to be translated to a usable number anyway as far as I can tell.

I wonder if we should specify an end of String character. Byte Zero maybe? Otherwise, how do you send multiple variably sized strings in the same payload.

Why would there be any strings other than what is defined?

Error conditions?
Can't think of more.

Still potentially a good idea. Other than the two predefined variable ones and the one fixed one that are agreed upon for int/vers/user use what is there. Now, what is there that would need to be sent with other data? interface and version will be sent in those predescribed packets only (doesn't make sense to do anything else). If there was an error string at all, it would come out alone in a packet too. I just can't see why. There may be reasons I haven't thought of though. That's why you guys are all here :-)

[*]Include extensive "basic" vars in part 1 spec for compatibility with simple devices that can't cope with flash protocols ;-) ?? (would require detailed thinking...)
Yeah, maybe. Perhaps instead of the STX byte 0xaa, you start with another byte, which proceeds with a text based interface. Do we need to worry about this now? Seems like work we dont need right now. Leave it for version 2?

Indeed, we don't need to worry about it now, provided that the facility for protocol payload types in bulk is there as agreed above, this would be easy to add later as an "extension". Just throwing ideas around. I also don't want to bother with it now, but I do want that bit flag for the payload type number to potentially allow it later.

[*]Acknowledgment form. I say empty = positive, full = stretchy int with error code of some sort. Thoughts?
Not sure really. It has a lot to do with how much processing capacity is available in the ECU, and the way the firmware is written.

I don't think that has to be the case. If the feature is present, then it is optional to use it. i.e. if it is a negative ack, just send a zero byte, and if possible, send an empty packet. in that way no HP is required and later if we want to use it for whatever reason, we can just start putting numbers in there. Same on the PC end, just check for a number, and if present, know it is negative, but no requirement to care why (just for logging perhaps).

Good work! :D

Cheers, glad you approve.

Admin.

[FlappySocks]

I wonder if we should specify an end of String character. Byte Zero maybe? Otherwise, how do you send multiple variably sized strings in the same payload.

Why would there be any strings other than what is defined?

Error conditions?
Can't think of more.

Neither can I really. Debug information in text form is always handy.

I feel it's just good practice more than anything else. Simple one byte char that terminates a variable length string. Allows you to add further data afterwards. If we dont do it, someone may curse us later.

Stu.