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.

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

Incorrect diagram drawn for custom protocol #9

Open
mcginty opened this issue Jun 20, 2019 · 3 comments
Open

Incorrect diagram drawn for custom protocol #9

mcginty opened this issue Jun 20, 2019 · 3 comments

Comments

@mcginty
Copy link

mcginty commented Jun 20, 2019

In this protocol, d is mis-drawn as 8 bits when it's specified as 16.

./protocol "a:64,b:8,c:16,d:16"
 0                   1                   2                   3
 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|                                                               |
+                               a                               +
|                                                               |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|       b       |               c               |       d       |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

Interestingly, if you do the following, it seems to draw correctly:

./protocol "a:64,b:8,c:16,d:16" -b 48
 0                   1                   2                   3                   4
 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|                                               a                                               |
+                               +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|                               |       b       |               c               |       d       |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|               |
+-+-+-+-+-+-+-+-+
@thibodux
Copy link

Seems like it is a more general issue with an even less complicated protocol that wraps the first field on the first two lines.

 ./protocol -b 16 "a:32,b:8"
 0                   1
 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|                               |
+               a               +
|                               |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

Shortening the first field to be exactly the same size as the width appears to work fine:

./protocol -b 16 "a:16,b:8"
 0                   1
 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|               a               |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|       b       |
+-+-+-+-+-+-+-+-+

@thibodux
Copy link

I think this is the simple fix with a test case. Not making a fork and MR for now cause it seems like other MRs are being ignored.

diff --git a/protocol b/protocol
index abbed87..696fe6c 100755
--- a/protocol
+++ b/protocol
@@ -431,6 +431,9 @@ class Protocol():
                             if i==lines_to_print-1:
                                 lines.append(self._get_horizontal())

+                        # increment field counter
+                        fields_done+=1
+
                 # Case 2: We are not at the beginning of the line and we need
                 # to print something that does not fit in the current line
                 else:
diff --git a/test.py b/test.py
index 105edc2..eb84f22 100755
--- a/test.py
+++ b/test.py
@@ -241,6 +241,19 @@ Urgent Pointer:16,Options:24,Padding:8",
 5                            Field_32                           5
 14343434343434343434343434343434343434343434343434343434343434342"""),

+("Field_64:64,Field_8:8?numbers=0,bits=16",
+"""+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|                               |
++                               +
+|                               |
++            Field_64           +
+|                               |
++                               +
+|                               |
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|    Field_8    |
++-+-+-+-+-+-+-+-+"""),
+
 ]

leytou added a commit to leytou/protocolpro that referenced this issue Aug 15, 2023
leytou added a commit to leytou/protocolpro that referenced this issue Aug 15, 2023
leytou added a commit to leytou/protocolpro that referenced this issue Aug 15, 2023
leytou added a commit to leytou/protocolpro that referenced this issue Aug 15, 2023
@leytou
Copy link

leytou commented Aug 15, 2023

Fixed, in my forked project: https://github.com/leytou/protocolpro
Or you can install it directly with the following command:
pip install protocolpro

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants