I concur that keeping an 8-byte header is desirable. If we will have only a very few message outstanding why not use 4 bits for the nh_seq. Although I feel 8-bits is probably enough message IDs for a given group (these are small devices after all) It would seem that we should save bits if they are not necessary. Maybe we can use 3-4 bits for nh_seq and reserve the rest of that byte.
Are we using flags? Since we are breaking compatibility do we want to re-order these fields and move the extra bits together into flags. Really, to give a proper review for this I¹d need to understand all the fields. Is there a doc describing this protocol? On 5/3/16, 2:49 PM, "marko kiiskila" <[email protected]> wrote: >Hi, > >I was going to add a sequence number to message header >to match responses to requests. It would be better if we >could detect responses to retransmitted requests, for example. > >I was going steal one byte from nh_id field and have one >byte worth of sequence number. I feel one byte being >sufficient width, as I don¹t think there would be that many >outstanding requests at any given time. Just one or two. > >I think keeping the header size as 8 bytes is valuable, as well as >keeping it a multiple of 4 bytes. > >Here¹s the old header: >struct nmgr_hdr { > uint8_t nh_op; > uint8_t nh_flags; > uint16_t nh_len; > uint16_t nh_group; > uint16_t nh_id; >}; > >Here is what the new header would look like: >struct nmgr_hdr { > uint8_t nh_op; /* NMGR_OP_XXX */ > uint8_t nh_flags; > uint16_t nh_len; /* length of the payload */ > uint16_t nh_group; /* NMGR_GROUP_XXX */ > uint8_t nh_seq; /* sequence number */ > uint8_t nh_id; /* message ID within group */ >}; > >This will break backwards compatibility, when requester starts >filling in the sequence number. > >Any objections, or other comments? > >I was also going to add a CRC at the end of the message, when >newtmgr goes over serial line. You can run this protocol over >UARTs with no flow control, so we should detect errors on this. >‹ >M
