stevedlawrence commented on a change in pull request #263: Add more obscure charsets needed for some mil formats. URL: https://github.com/apache/incubator-daffodil/pull/263#discussion_r312541661
########## File path: daffodil-io/src/main/scala/org/apache/daffodil/processors/charset/X_DFDL_MIL_STD.scala ########## @@ -48,6 +48,46 @@ object BitsCharset6BitDFI311DUI002 extends { override val requiredBitOrder = BitOrder.LeastSignificantBitFirst } with BitsCharsetNonByteSize +object BitsCharset3BitDFI336DUI001 extends { + override val name = "X-DFDL-3-BIT-DFI-336-DUI-001" + override val bitWidthOfACodeUnit = 3 + override val decodeString = """12345678""" + override val replacementCharCode = 0x0 + override val requiredBitOrder = BitOrder.LeastSignificantBitFirst +} with BitsCharsetNonByteSize + +object BitsCharset4BitDFI746DUI002 extends { + override val name = "X-DFDL-4-BIT-DFI-746-DUI-002" + override val bitWidthOfACodeUnit = 4 + override val decodeString = """ABCDEFGHIJKLMNPQ""" + override val replacementCharCode = 0x0 + override val requiredBitOrder = BitOrder.LeastSignificantBitFirst +} with BitsCharsetNonByteSize + +object BitsCharset3BitDFI746DUI002 extends { + override val name = "X-DFDL-3-BIT-DFI-746-DUI-002" + override val bitWidthOfACodeUnit = 3 + override val decodeString = """ABCDEFGH""" + override val replacementCharCode = 0x0 + override val requiredBitOrder = BitOrder.LeastSignificantBitFirst +} with BitsCharsetNonByteSize + +object BitsCharset3BitDFI747DUI001 extends { + override val name = "X-DFDL-3-BIT-DFI-747-DUI-001" Review comment: Minor and probably not worth changing, but I've never been a fan of charset names that contain DFI/DUI's. It seems so specific to a single use case of a charset. Granted most of them probably are the only cases, but still seems odd. Especially since encodings *could* be shared by by different DFI/DUIs. For example, maybe DFI 123 DUI 456 has the same encoding. Using this encoding for that DFI/DUI now will look like a bug in the schema, even though it's would be correct. But maybe the solution there is to just add aliases? I'd maybe rather see something like X-DFDL-TDL-NNNN-M, where NNNN a zero-padded number that we increment for every new TDL encoding we come across, and M is the bit length. So this might be X-DFDL-TDL-0010-3. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services