Sorry for the late response… this looks good to me.
> On May 3, 2016, at 2:49 PM, marko kiiskila <ma...@runtime.io> 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
