Fix incorrect specification of directory contents in read dir resp [A…

…P-945] (#1379)

# Description

@swift-nav/devinfra

MSG_FILEIO_READ_DIR_RESP has a string field which indicates the contents
of the directory. In the SBP spec the encoding of the string is set
properly (multipart) but the type of the field is incorrectly set to
array. This means that the field is not actually treated as a string in
the generated bindings but rather as a byte array.

This generally isn’t causing problems because this message is only used
by piksi and related tools which are all using older versions of libsbp.
They generally haven’t been updated to use the V4 API so this change
will not actually cause any problems. But it should be fixed to enable
improvements to unit tests

# API compatibility

Technically this does introduce an API change, but the message is unused
except in legacy contexts so there should be no issue at all

## API compatibility plan

If the above is "Yes", please detail the compatibility (or migration)
plan:

Should any issues come up they are easy to resolve, simply use the newly
generated functions in the V4 API
(`sbp_msg_fileio_read_dir_resp_contents_*`) instead of directly
interacting with the byte array

# JIRA Reference

https://swift-nav.atlassian.net/browse/AP-945

c/include/libsbp/legacy/file_io.h

@@ -97,8 +97,8 @@ typedef struct SBP_ATTR_PACKED {
  */
 
 typedef struct SBP_ATTR_PACKED {
-  u32 sequence;   /**< Read sequence number */
-  u8 contents[0]; /**< Contents of read directory */
+  u32 sequence;     /**< Read sequence number */
+  char contents[0]; /**< Contents of read directory */
 } msg_fileio_read_dir_resp_t;
 
 /** Delete a file from the file system (host => device)

haskell/sbp.cabal

@@ -173,4 +173,4 @@ test-suite test
                      , tasty
                      , tasty-hunit
   ghc-options:         -threaded -rtsopts -with-rtsopts=-N -Wall
-  default-language:    Haskell2010
+  default-language:    Haskell2010

haskell/src/SwiftNav/SBP/FileIo.hs

@@ -165,19 +165,19 @@ msgFileioReadDirResp = 0x00AA
 data MsgFileioReadDirResp = MsgFileioReadDirResp
   { _msgFileioReadDirResp_sequence :: !Word32
     -- ^ Read sequence number
-  , _msgFileioReadDirResp_contents :: ![Word8]
+  , _msgFileioReadDirResp_contents :: !Text
     -- ^ Contents of read directory
   } deriving ( Show, Read, Eq )
 
 instance Binary MsgFileioReadDirResp where
   get = do
     _msgFileioReadDirResp_sequence <- getWord32le
-    _msgFileioReadDirResp_contents <- whileM (not <$> isEmpty) getWord8
+    _msgFileioReadDirResp_contents <- decodeUtf8 . toStrict <$> getRemainingLazyByteString
     pure MsgFileioReadDirResp {..}
 
   put MsgFileioReadDirResp {..} = do
     putWord32le _msgFileioReadDirResp_sequence
-    mapM_ putWord8 _msgFileioReadDirResp_contents
+    putByteString $ encodeUtf8 _msgFileioReadDirResp_contents
 
 $(makeSBP 'msgFileioReadDirResp ''MsgFileioReadDirResp)
 $(makeJSON "_msgFileioReadDirResp_" ''MsgFileioReadDirResp)

java/src/com/swiftnav/sbp/file_io/MsgFileioReadDirResp.java

@@ -16,7 +16,6 @@
 
 import com.swiftnav.sbp.SBPBinaryException;
 import com.swiftnav.sbp.SBPMessage;
-import org.json.JSONArray;
 import org.json.JSONObject;
 
 /**
@@ -37,7 +36,7 @@ public class MsgFileioReadDirResp extends SBPMessage {
     public long sequence;
 
     /** Contents of read directory */
-    public int[] contents;
+    public String contents;
 
     public MsgFileioReadDirResp(int sender) {
         super(sender, TYPE);
@@ -58,20 +57,20 @@ public MsgFileioReadDirResp(SBPMessage msg) throws SBPBinaryException {
     protected void parse(Parser parser) throws SBPBinaryException {
         /* Parse fields from binary */
         sequence = parser.getU32();
-        contents = parser.getArrayofU8();
+        contents = parser.getString();
     }
 
     @Override
     protected void build(Builder builder) {
         builder.putU32(sequence);
-        builder.putArrayofU8(contents);
+        builder.putString(contents);
     }
 
     @Override
     public JSONObject toJSON() {
         JSONObject obj = super.toJSON();
         obj.put("sequence", sequence);
-        obj.put("contents", new JSONArray(contents));
+        obj.put("contents", contents);
         return obj;
     }
 

javascript/sbp/file_io.js

@@ -154,7 +154,7 @@ MsgFileioReadDirReq.prototype.fieldSpec.push(['dirname', 'string', null]);
  *
  * Fields in the SBP payload (`sbp.payload`):
  * @field sequence number (unsigned 32-bit int, 4 bytes) Read sequence number
- * @field contents array Contents of read directory
+ * @field contents string Contents of read directory
  *
  * @param sbp An SBP object with a payload to be decoded.
  */
@@ -172,10 +172,10 @@ MsgFileioReadDirResp.prototype.constructor = MsgFileioReadDirResp;
 MsgFileioReadDirResp.prototype.parser = new Parser()
   .endianess('little')
   .uint32('sequence')
-  .array('contents', { type: 'uint8', readUntil: 'eof' });
+  .string('contents', { greedy: true });
 MsgFileioReadDirResp.prototype.fieldSpec = [];
 MsgFileioReadDirResp.prototype.fieldSpec.push(['sequence', 'writeUInt32LE', 4]);
-MsgFileioReadDirResp.prototype.fieldSpec.push(['contents', 'array', 'writeUInt8', function () { return 1; }, null]);
+MsgFileioReadDirResp.prototype.fieldSpec.push(['contents', 'string', null]);
 
 /**
  * SBP class for message MSG_FILEIO_REMOVE (0x00AC).

kaitai/ksy/file_io.ksy

@@ -114,8 +114,9 @@ types:
       - id: contents
         doc: |
           Contents of read directory
-        type: u1
-        repeat: eos
+        type: str
+        encoding: ascii
+        size-eos: true
 
   msg_fileio_remove:
     doc: |

kaitai/ksy/sbp.ksy

@@ -590,4 +590,4 @@ types:
             2307: vehicle::msg_odometry
             2308: vehicle::msg_wheeltick
       - id: crc
-        type: u2
+        type: u2

proto/file_io.proto

@@ -84,7 +84,7 @@ message MsgFileioReadDirReq {
  */
 message MsgFileioReadDirResp {
     uint32 sequence = 1;
-    repeated uint32 contents = 2;
+    string contents = 2;
 }
 
 /** Delete a file from the file system (host => device)

python/sbp/file_io.py

@@ -384,15 +384,15 @@ class MsgFileioReadDirResp(SBP):
     SBP parent object to inherit from.
   sequence : int
     Read sequence number
-  contents : array
+  contents : string
     Contents of read directory
   sender : int
     Optional sender ID, defaults to SENDER_ID (see sbp/msg.py).
 
   """
   _parser = construct.Struct(
                    'sequence' / construct.Int32ul,
-                   'contents' / construct.GreedyRange(construct.Int8ul),)
+                   'contents' / construct.GreedyBytes,)
   __slots__ = [
                'sequence',
                'contents',

rust/sbp/src/messages/file_io.rs

@@ -358,7 +358,7 @@ pub mod msg_fileio_read_dir_resp {
         pub sequence: u32,
         /// Contents of read directory
         #[cfg_attr(feature = "serde", serde(rename = "contents"))]
-        pub contents: Vec<u8>,
+        pub contents: SbpString<Vec<u8>, Multipart>,
     }
 
     impl ConcreteMessage for MsgFileioReadDirResp {
@@ -407,7 +407,8 @@ pub mod msg_fileio_read_dir_resp {
     }
 
     impl WireFormat for MsgFileioReadDirResp {
-        const MIN_LEN: usize = <u32 as WireFormat>::MIN_LEN + <Vec<u8> as WireFormat>::MIN_LEN;
+        const MIN_LEN: usize =
+            <u32 as WireFormat>::MIN_LEN + <SbpString<Vec<u8>, Multipart> as WireFormat>::MIN_LEN;
         fn len(&self) -> usize {
             WireFormat::len(&self.sequence) + WireFormat::len(&self.contents)
         }

spec/yaml/swiftnav/sbp/file_io.yaml

@@ -114,7 +114,7 @@ definitions:
           type: u32
           desc: Read sequence number
       - contents:
-          type: array
+          type: string
           encoding: multipart
           fill: u8
           desc: Contents of read directory