Simple defbinary and also example from wiki fail to compile

[dg1sbg]

Hi all:

The following code results in an error:

(defbinary packet-header (:export t :byte-order :big-endian) ((packet-type class-identifier-p indicators tsi tsf packet-count packet-size) 0 :type (bit-field :raw-type (unsigned-byte 32) :member-types ((unsigned-byte 4) (unsigned-byte 1) (unsigned-byte 3) (unsigned-byte 4) (unsigned-byte 16)))))

=>
In bitfield: Number of values 7 (PACKET-TYPE CLASS-IDENTIFIER-P INDICATORS TSI TSF PACKET-COUNT PACKET-SIZE) doesn't match number of types 0 NIL

I then tried to compile the example in the defbinary wiki page with a similar result. I also tried diiferent Lisp implementations with same results.

What am I doing wrong? Thanks for any hints!

Best,
Frank

[dg1sbg]

@j3pic Could you please follow up with a hint or else a short reply indicating if you have time at all to look into this. Thank you so much for this great library and also for feedback.

Kind regards
Frank

[j3pic]

I tried your code example and got a different error from you:

Member types ((UNSIGNED-BYTE 4) (UNSIGNED-BYTE 1)
              (UNSIGNED-BYTE 3) (UNSIGNED-BYTE 4)
              (UNSIGNED-BYTE 16)) don't add up to 32 bits

I reindented your code and got this:

(defbinary packet-header (:export t :byte-order :big-endian)
  ((packet-type
    class-identifier-p
    indicators
    tsi tsf
    packet-count
    packet-size) 0
   :type (bit-field :raw-type (unsigned-byte 32)
		    :member-types ((unsigned-byte 4)
				   (unsigned-byte 1)
				   (unsigned-byte 3)
				   (unsigned-byte 4)
				   (unsigned-byte 16)))))

The problem is that the member-types only add up to 28 bits, but you specified that the whole bit field takes up
32 bits.

I tried to rewrite this struct using automatic bit-field conversion, but you didn't supply enough field sizes for
packet-size and packet-count. The declaration I came up with looks like this:

(defbinary packet-header (:export t :byte-order :big-endian)
  (packet-type 0 :type (unsigned-byte 4))
  (class-identifier-p 0 :type (unsigned-byte 1))
  (indicators 0 :type (unsigned-byte 3))
  (tsi 0 :type (unsigned-byte 4))
  (tsf 0 :type (unsigned-byte 16)))
 ;; TODO - add packet-count and packet-size

That compiled without issue.

[dg1sbg]

Thank you so much for looking into this. I was puzzled that you got the second form to compile as I still get the same error - so I kept digging deeper. For this, I switched to the github repo and cloned branch master to have the most up-to-date code.

I looked into the defbinary macro and tried to identify where things are going wrong for me.

With:

(setq *field-descriptions*
  '((PACKET-TYPE        0 :TYPE (UNSIGNED-BYTE 4))
    (CLASS-IDENTIFIER-P 0 :TYPE (UNSIGNED-BYTE 1))
    (INDICATORS         0 :TYPE (UNSIGNED-BYTE 3))
    (TSI                0 :TYPE (UNSIGNED-BYTE 2))
    (TSF                0 :TYPE (UNSIGNED-BYTE 2))
    (PACKET-COUNT       0 :TYPE (UNSIGNED-BYTE 4))
    (PACKET-SIZE        0 :TYPE (UNSIGNED-BYTE 16))))
                           ;; This now adds up to 32 bit...

This is turned into fields being

(#S(BINARY-FIELD :NAME (PACKET-TYPE CLASS-IDENTIFIER-P INDICATORS)
                 :DEFSTRUCT-FIELD ((PACKET-TYPE CLASS-IDENTIFIER-P INDICATORS)
                                   (0 0 0)
                                   :TYPE NIL)
                 :READ-FORM NIL
                 :WRITE-FORM NIL
                 :TYPE (BIT-FIELD :MEMBER-TYPES ((UNSIGNED-BYTE 4)
                                                 (UNSIGNED-BYTE 1)
                                                 (UNSIGNED-BYTE 3))
                                  :RAW-TYPE (UNSIGNED-BYTE 8))
                 :BIT-STREAM-ID NIL)
 #S(BINARY-FIELD :NAME (TSI TSF PACKET-COUNT)
                 :DEFSTRUCT-FIELD ((TSI TSF PACKET-COUNT)
                                   (0 0 0)
                                   :TYPE NIL)
                 :READ-FORM NIL
                 :WRITE-FORM NIL
                 :TYPE (BIT-FIELD :MEMBER-TYPES ((UNSIGNED-BYTE 2)
                                                 (UNSIGNED-BYTE 2)
                                                 (UNSIGNED-BYTE 4))
                                  :RAW-TYPE (UNSIGNED-BYTE 8))
                               :BIT-STREAM-ID NIL)
 #S(BINARY-FIELD :NAME PACKET-SIZE
                 :DEFSTRUCT-FIELD (PACKET-SIZE 0 :TYPE NIL)
                 :READ-FORM NIL
                 :WRITE-FORM NIL
                 :TYPE (UNSIGNED-BYTE 16)
                 :BIT-STREAM-ID NIL))

And with this I found that the following part of defbinary seems to not work for me:

LISP-BINARY>  `(defstruct packet-header
                       ,@(loop for (name default-value . options) in (mapcar #'binary-field-defstruct-field *fields*)
                               for type = (getf options :type) ;; <<<=== THIS SEEMS TO NOT WORK ...
                               if (listp name)                 ;; <<<=== (listp name) -> t   here, so we enter the append ...
                                 append (bitfield-spec->defstruct-specs name default-value options nil)
                               else collect (list* name default-value :type (if nil t
                                                                                type)
                                                   (remove-plist-keys options :type :bit-stream-id))))

At this point I could use some help as I can't find the code that sets up the plist of a field to comtain type information propery for bit-fields ...

I also attached a screenshot of my debugging session ...

Could you please look into this? A million thanks!

Best,
Frank

[j3pic]

The plist is extracted directly from the type definition.

I find that this evaluates without any problems:

(defbinary packet-header (:export t :byte-order :little-endian)
  (PACKET-TYPE        0 :TYPE (UNSIGNED-BYTE 4))
  (CLASS-IDENTIFIER-P 0 :TYPE (UNSIGNED-BYTE 1))
  (INDICATORS         0 :TYPE (UNSIGNED-BYTE 3))
  (TSI                0 :TYPE (UNSIGNED-BYTE 2))
  (TSF                0 :TYPE (UNSIGNED-BYTE 2))
  (PACKET-COUNT       0 :TYPE (UNSIGNED-BYTE 4))
  (PACKET-SIZE        0 :TYPE (UNSIGNED-BYTE 16)))

I notice one difference between our two systems that may account for the problem: You are using Allegro Common Lisp. That's a commercial Lisp, and since they make you e-mail them to get prices, I take that as a sign that a license costs thousands or even tens of thousands of dollars: Out of my price range either way.

I have only ever tested this library on open-source Lisp implementations, such as SBCL, CCL, and CLISP.

If you have a PR that fixes this bug without breaking things for the open-source Lisps, I'll merge it.

Also, have you seen this wiki page? https://github.com/j3pic/lisp-binary/wiki/Encoding-and-Parsing-Bit-Strings

[j3pic]

I took a closer look at your code snippets and the screen shot. The problem first appears here:

 #S(BINARY-FIELD :NAME PACKET-SIZE
                 :DEFSTRUCT-FIELD (PACKET-SIZE 0 :TYPE NIL)
                 :READ-FORM NIL
                 :WRITE-FORM NIL
                 :TYPE (UNSIGNED-BYTE 16)
                 :BIT-STREAM-ID NIL))

The problem is the defstruct field specification (PACKET-SIZE 0 :TYPE NIL). That gets inserted directly into a cl:defstruct form, which will be rejected because NIL is not a valid Common Lisp type. I don't know how you ended up with that. I tried this and got something else:

LISP-BINARY-TEST> (apply #'lisp-binary::expand-defbinary-field 
			 (append '(PACKET-SIZE        0 :TYPE (UNSIGNED-BYTE 16))
				 '(:byte-count-name :byte-count
				   :stream-symbol :stream)))
#S(LISP-BINARY::BINARY-FIELD
   :NAME PACKET-SIZE
   :DEFSTRUCT-FIELD (PACKET-SIZE 0 :TYPE (UNSIGNED-BYTE 16))
   :READ-FORM (READ-INTEGER 2 :STREAM :BYTE-ORDER :LITTLE-ENDIAN :SIGNED NIL
                            :SIGNED-REPRESENTATION :TWOS-COMPLEMENT)
   :WRITE-FORM (WRITE-INTEGER PACKET-SIZE 2 :STREAM :BYTE-ORDER :LITTLE-ENDIAN
                              :SIGNED-REPRESENTATION :TWOS-COMPLEMENT :SIGNED
                              NIL)
   :TYPE (UNSIGNED-BYTE 16)
   :BIT-STREAM-ID NIL)

[j3pic]

Another weird anomaly is that none of your fields are having read or write forms generated for them. You see in my example above that I didn't end up with NIL for those fields. But you got NIL for all your read and write forms. That's a critical failure because those forms get inserted directly into the read-binary and write-binary methods. If you're getting NIL during field expansion, the methods would do nothing, even if you got a valid defstruct form out of it.

[dg1sbg]

Hi - thanks so much for looking into this. I finally found the time to actually get to the root cause - changed two lines in binary-2.lisp and voilà, it works. I created PR #72: #72 Please review and, if ok, merge. Sincere thanks for helping me here! Regards Frank a.k.a dg1sbg Amateur Radio Call DG1SBG

…

Am 06.03.2024 um 09:42 schrieb j3pic ***@***.***>: I took a closer look at your code snippets and the screen shot. The problem first appears here: #S(BINARY-FIELD :NAME PACKET-SIZE :DEFSTRUCT-FIELD (PACKET-SIZE 0 :TYPE NIL) :READ-FORM NIL :WRITE-FORM NIL :TYPE (UNSIGNED-BYTE 16) :BIT-STREAM-ID NIL)) The problem is the defstruct field specification (PACKET-SIZE 0 :TYPE NIL). That gets inserted directly into a cl:defstruct form, which will be rejected because NIL is not a valid Common Lisp type. I don't know how you ended up with that. I tried this and got something else: CL-USER>(lisp-binary::expand-defbinary-field 'PACKET-TYPE 0 :TYPE '(UNSIGNED-BYTE 4) :byte-count-name :bc :stream-symbol :stream) #S(LISP-BINARY::BINARY-FIELD :NAME PACKET-TYPE :DEFSTRUCT-FIELD (PACKET-TYPE 0 :TYPE (UNSIGNED-BYTE 4)) :READ-FORM (READ-INTEGER 1/2 :STREAM :BYTE-ORDER :LITTLE-ENDIAN :SIGNED NIL :SIGNED-REPRESENTATION :TWOS-COMPLEMENT) :WRITE-FORM (WRITE-INTEGER PACKET-TYPE 1/2 :STREAM :BYTE-ORDER :LITTLE-ENDIAN :SIGNED-REPRESENTATION :TWOS-COMPLEMENT :SIGNED NIL) :TYPE (UNSIGNED-BYTE 4) :BIT-STREAM-ID NIL) — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: ***@***.***>

[joelreymont]

The above PR should do it.

[j3pic]

Fixed.