Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Packet feedback 2024 08 12 and general packet additions #47

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Packet feedback 2024 08 12 and general packet additions #47

wants to merge 14 commits into from

Conversation

sthagen
Copy link
Contributor

@sthagen sthagen commented Aug 15, 2024 •
edited
Loading

  • corrected offset calculations in connect packet
  • harmonized the parameter for offset notation in commented packets
  • increased the font in disconnect flags section for readability
  • added diagrams for general packet parts
  • removed directory with few alternate example diagram formats to reduce distractions

@sthagen
Removed alternate packet diagram directory
9423baf 
Signed-off-by: Stefan Hagen <stefan@hagen.link>
@sthagen
Implemented feedback from 2024-08-12 by Davide
c9caa76 
- corrected offset calculations in connect packet
- harmonized the parameter for offset notation in connect,
  disconnect, pingreq, pingresp, and unsubscribe-with-topic-filter  packets
- increased the font in disconnect flags section for readability

Signed-off-by: Stefan Hagen <stefan@hagen.link>
@sthagen sthagen added documentation Improvements or additions to documentation editorial Mostly editorial changes. labels Aug 15, 2024
@sthagen sthagen self-assigned this Aug 15, 2024
@sthagen
Added header packet part with both formats
40cdedc 
Signed-off-by: Stefan Hagen <stefan@hagen.link>
@sthagen sthagen marked this pull request as ready for review August 15, 2024 18:46
@sthagen
Implemented feedback on auth packet too (google docs comments are har…
4e1ed61 
…d to navigate at that size, sorry)

Signed-off-by: Stefan Hagen <stefan@hagen.link>
@sthagen
Fixed register too and dito
a0a549b 
Signed-off-by: Stefan Hagen <stefan@hagen.link>
@sthagen
Fixed pubwos too and dito
f2cd20a 
Signed-off-by: Stefan Hagen <stefan@hagen.link>
@sthagen
Fixed publish-qos-zero too and dito
0c041ac 
Signed-off-by: Stefan Hagen <stefan@hagen.link>
@sthagen
Fixed publish-qos-1-and-2 too and dito
f0c6934 
Signed-off-by: Stefan Hagen <stefan@hagen.link>
@sthagen
Fixed subscribe-with-topic-filter too and dito
f6d9b94 
Signed-off-by: Stefan Hagen <stefan@hagen.link>
@sthagen
Nit: fontsize of length/position indicator
5f9a426 
Signed-off-by: Stefan Hagen <stefan@hagen.link>
@sthagen
Fixed forwarder-encapsulation too and dito
9606ced 
Signed-off-by: Stefan Hagen <stefan@hagen.link>
@sthagen
Added utf-8 encoded string diagram
09d9eb8 
Signed-off-by: Stefan Hagen <stefan@hagen.link>
@sthagen
Added utf-8 string example 1
634c8a1 
Signed-off-by: Stefan Hagen <stefan@hagen.link>
@sthagen
Added glyphs of utf-8 values in example diagram
5a81c00 
Signed-off-by: Stefan Hagen <stefan@hagen.link>
@lenzarda
Copy link
Contributor

Connect packet:

  • I think S should be =R+3+L to be consistent with the above field (as R=P+1)
  • I think U should be =S+N to be consistent with the above field (as T=S+1)

lenzarda
lenzarda requested changes Aug 28, 2024
View reviewed changes
Copy link
Contributor

@lenzarda lenzarda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Connect packet:

I think S should be =R+3+L to be consistent with the above field (as R=P+1)
I think U should be =S+N to be consistent with the above field (as T=S+1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lenzarda lenzarda lenzarda requested changes

@icraggs icraggs Awaiting requested review from icraggs

Assignees

@sthagen sthagen

Labels
documentation Improvements or additions to documentation editorial Mostly editorial changes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sthagen @lenzarda