Welcome along Ed :-)
Perhaps I should have marked 0.4 as out of date too :-)
EdG wrote:Reading the earlier parts, I was going to suggest a 'payload length' *instead* of escaped characters in the payload. I see there is a payload length in the header, but still the esc. characters. Why use esc. characters at all? They are only of discriminatory use if they actually appear in the data, and they could potentially double payload sizes. The checksum & end byte appearing in the correct place should be enough, and that way you get predictable packet sizes - useful if these packets will be wrapped & sent over other comms. devices/channels in the future...
The idea is to simplify the reception process and allow it to be layered. The size increase *could* be double, but more likely it will be about (256 + 3) / 256 in practice. The escapes never make it inside to the buffer or out to another protocol, they only exist on the physical line and though the first layer. It also adds another mode of error checking in that if you have an escape appear and you flag the next byte as being escaped, and it isn't one of the three escaped bytes possible, then you can drop reception and start looking for a start byte again. Also, how would you plan to know when you were starting reception and not? timestamps and time outs? This is irrespective of timing. You simply look for start, when you find it you start receiving. If you find an escape you set a flag, when the next char comes you convert it and store it, if it doesn't match you reset. The length can be checked once the packet has been fully received. Ditto the checksum. I think there would be too much stuff being done in the ISR to base it on header contents up front. As it stands its quite minimal and will get a complete packet into the buffer with minimal ISR overhead. Once it is there a flag can be set and the main loop processing code can deal with it.
I see a lot of pluses for Stu's way, and the 1.5% hit in speed seems negligible to me. I'm open to being convinced, but packet sizes will be consistent in the buffer and for forwarding, and the speed hit is minimal. Thoughts?
I'd be tempted to consider a 16 bit checksum if the packets are going to be long. Maybe for larger packets, and potentially more critical data, additional checksumming could be included inside the payload?
With the parity bit on each byte, the escape scheme, the length, the length to ID relationship, and the 8 bit checksum, the chances of getting bad data have to be pretty slim don't they? As for inside the payload, sure, that is a possibility, but the point of checksumming at all is to have transparent reliable comms for the application layer. (I've been converted to this layer terminology since starting to actually code it)
Also, checksum header & payload. For example, what if just 'payload type' were corrupted. Can't think of any reason not to do this.
I can't think of one not to do it either, but then again, I can't think of one to do it, and it will cost an extra byte (not that that matters). The worst case for the above is that you listen to one message for too long before realising it's bad. Even that is slim to none type stuff as the next message will have a start byte at the beginning and cause a reset as soon as it starts anyway. I understand the limitation of the 8 bit checksum, but I'm not convinced we need any more. I had a hard enough time convincing some people that we needed a checksum and packetising etc at all. I see it as important, but I don't want 100% certainty that the data IS correct, just a 99.9% is ok by me. The speed and complexity hits for more complex and more checksums seem to outweigh any benefits for the small % cases that will get through the other checks.
Pre/post ESCed payload? Doesn't really matter I suppose, though I'd probably tend to C'sum all the actual bytes which are transferred over the comms link - that's the purpose of the c'sum.
I wasn't sure either, but thinking about it now the escapes will check themselves anyway and doing it preescaping means you can run the checksum and store it before transmission (I'm not doing it that way, but you could.)
Stretchable integer - this looks like an unnecessary complication to me.
It's gone already.
A few places you mentioned larger packet sizes. As it stands there is no requirement to deliver more than about 1k of data at a time. There isn't the buffer to do it, and for local stuff the flash block granularity is 1k. So sizes aren't an issue for us, 16 bits is heaps, and I think 60k of data should be enough per packet for anything really (famous last words). I base that on the fact that bigger stuff could be broken up into chunks quite easily if required.
Does the 'ACK byte present' bit in the Header Flags indicate that an ACK is required in response to this message? Or is an ACK always required (hence the fixed ACK number byte).
Acks were a work in progress. It's open for debate as is everything else :-)
Currently what I have is the following :
When a device wishes to receive confirmation that what it asked for was actioned appropriately and indeed whether the outcome was successful or not it should set the header bit for acknowledgement valid/required, and populate the acknowledgement field with the next value in the sequence being used. After processing the request the receiving device will respond with the correct payload type/id, the matching acknowledgement number, and the flag set to either success of failure. In the case of failure the body, if present, will be interpretted as an error code. In the case of success the body will either be empty or contain the requested data.
That uses two 1 bit flags in the header and the 8 bit ack. I decided to leave all fields always present to simplify things a little more. That could be subject to change yet, we'll see how it looks when I start to code up the packet handling stuff.
The thing is that it is 1 byte from 8 (with addresses) and it is the only one that is optional. I tend to think it should stay just to keep the locations of the other aspects consistent. IE, payload always starts at buffer start + 8 etc.
Hardware control I would say isn't really necessary, and not commonly used.
It's gone. To keep it atomic and simple the ecu will just turn off the receive isr while handling a packet and turn it back on after it's done. This avoids funky stuff happening on the ECU side which is the main concern.
I tend to feel that the tuning authors shouldn't be *able* to screw up the insides of the ecu. Hence the push towards a defined interface for most stuff. In this case, if they were to flood the line with messages, you would get the first one and start dealing with it and be blissfully ignorant of the rest. The tuning tool would realise it didn't get what it wanted and try again at some point.
For this eventuality, it might be worth defining maximum response times to requests so sensible timeouts can be used. Just something to add to the docs once it's coded, not something to define now.
Good point, I'll add in an empty section now as a reminder. Thanks!
Having said that, the other option - a reported maximum - isn't really any overhead (though possibly unnecessary) so I'd vote for that. It's there if needed, or can be ignored.
Exactly, that is the current state of the play for that. A given firmware will define its individual message types as certain sizes and it then only matters for forwarding stuff to CAN etc. So the facility exists to ask for it, but it won't be used for normal stuff at all.
As for the CAN bus discussions, I agree - disregard it for now.
I'm happy to do that with the single exception that we need to be able to pass data through to it. Perhaps that should be part of the firmware part of the document rather than an integral protocol function. I just don't want to limit ourselves for that by making a bad decision now.
With regards addresses in the serial spec, I realise it should be point to point, and I was urring on the no addresses side too, but the way delta put it, it's very straight forward to do it right, and it allows misuse of the serial lines by paralleling other devices up. I tend to think they should stay. Again, I'm open to discussion, but we really need to nail this down a bit more firmly very soon. I'll try to get the document released later today, but no promises on that. I've already made too many as it stands!
What do I expect from CAN :
Share runtime data with other nodes
Configure other nodes through one serial connection
Perhaps command other nodes to perform actions, but this shouldn't be required, just program it to perform the action based on some data and then just send the data right?
Jared,
and ASCII for human readable data scrolling by a prompt.
That just isn't going to happen as it's not at all practical for a simple AND reliable setup. MS is a mixed bag, but has no checksums no packetising, you just cross your fingers and hope for the best. Once you ditch that paradigm you are going to lose the ASCII fairly quickly. The overhead isn't just for conversions, it's for bandwidth as well because to represent 65000 you need 5 bytes, but really it was only 2 ditto 255 is 3 instead of 1 and 4294967296 is 10 instead of 4.
As for your sensor data 2 byte thing, the data itself is 2 bytes, and there are more than 255 different RT variables or will be soon anyway.
It should be trivial to dump this packetised data out to the console through a processor, if you wanna see it go by, why not see it go by and be sure it is good. What is it that you have against a simple console tool like that?
Fred.