protocol icon indicating copy to clipboard operation
protocol copied to clipboard

Incorrect diagram drawn for custom protocol

Open mcginty opened this issue 5 years ago • 3 comments

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       |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|               |
+-+-+-+-+-+-+-+-+

mcginty avatar Jun 20 '19 08:06 mcginty

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 avatar Jun 28 '21 15:06 thibodux

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    |
++-+-+-+-+-+-+-+-+"""),
+
 ]

thibodux avatar Jun 28 '21 18:06 thibodux

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

leytou avatar Aug 15 '23 04:08 leytou