Remove JER whitespace

[mtfurlan]

It was wrongly formatted, and I don't think it's helpful to format JSON until you want to display it anyway.

Old behavior:

{
"MessageFrame":{

    "messageId": 32,
    "value": {
"PersonalSafetyMessage":{

            "basicType": "unavailable",
            "secMark": 65535,
            "msgCnt": 105,
            "id": "5D D7 5A 8C",
            "position": {

                "lat": 900000001,
                "long": 1800000001,
                "elevation": -4096}
            ,
            "accuracy": {

                "semiMajor": 255,
                "semiMinor": 255,
                "orientation": 65535}
            ,
            "speed": 8191,
            "heading": 28800,
            "accelSet": {

                "long": 2001,
                "lat": 2001,
                "vert": -127,
                "yaw": 0}
            ,
            "pathHistory": {

                "initialPosition": {

                    "utcTime": {

                        "year": 0,
                        "month": 0,
                        "day": 0,
                        "hour": 31,
                        "minute": 60,
                        "second": 65535,
                        "offset": 0}
                    ,
                    "long": 1800000001,
                    "lat": 900000001,
                    "elevation": -4096,
                    "posAccuracy": {

                        "semiMajor": 255,
                        "semiMinor": 255,
                        "orientation": 65535}
                    }
                ,
                "crumbData": [
                    {"PathHistoryPoint":{

                        "latOffset": -131072,
                        "lonOffset": -131072,
                        "elevationOffset": -2048,
                        "timeOffset": 65535}
                    }
                ]}
            }
        }}
}

New behavior

{"MessageFrame":{"messageId": 32,"value": {"PersonalSafetyMessage":{"basicType": "unavailable","secMark": 65535,"msgCnt": 7,"id": "5D D7 5A 8C","position": {"lat": 900000001,"long": 1800000001,"elevation": -4096},"accuracy": {"semiMajor": 255,"semiMinor": 255,"orientation": 65535},"speed": 8191,"heading": 28800,"accelSet": {"long": 2001,"lat": 2001,"vert": -127,"yaw": 0},"pathHistory": {"initialPosition": {"utcTime": {"year": 0,"month": 0,"day": 0,"hour": 31,"minute": 60,"second": 65535,"offset": 0},"long": 1800000001,"lat": 900000001,"elevation": -4096,"posAccuracy": {"semiMajor": 255,"semiMinor": 255,"orientation": 65535}},"crumbData": [{"PathHistoryPoint":{"latOffset": -131072,"lonOffset": -131072,"elevationOffset": -2048,"timeOffset": 65535}}]}}}}}

[mouse07410]

1. JSON file formatting. You may (or may not!) have a point: it's probably not helpful to format JSON unless you're displaying it. I'm not so sure of it - if I stored JSON into a file - I probably would not like it to be a huge single string.
2. Wrong length of the callbacks. This probably needs attending to.
3. Somehting behind `if(0). As I wasn't the one who wrote JER support, I can't tell you if it's a mistake or not. :-(. Perhaps, @spfoos would care to comment?

I propose splitting your PR into small(er) independent pieces. E.g., fixing ASN__CALLBACK does not need to go together with file formatting?

[mtfurlan]

Splitting this makes sense, I shouldn't have added the lengths issue to this PR when I found it.

I left my ASN.1 spec reading notes below, but it looks like the ASN.1 JER encoding rules don't specify whitespace so we can do whatever we want. I'm of the opinion that JSON should be minified during transport and only prettified for human consumption at the end. If I wrote it to a file I would prefer it to be minified so I can easily select and copy it when it fits on one screen, and if I want it prettified it I can just cat $file | jq Bytes may nearly be free, but not quite free so no reason to be wasteful.

If you feel strongly that we want both could maybe have JER and JER-WHITESPACE as different encodings the way we have two encodings for XER, though that is kinda wonky because the XER ones have ASN.1 defined differences where these would just be implementation defined.

---

JER is based on the XER implementation, and it looks like you can choose BASIC-XER or CANONICAL-XER, and it only adds newlines and indentation if it is not in CANONICAL-XER mode. I don't really follow what the spec for XER is saying, but the whitespace notes in basic XER say whitespace is permitted in some cases, but for canonical it is forbidden in several cases, which is what asn1c is doing.

The JER spec maybe says whitespace "can appear between lexical items" (but might be referencing something specific and not just all json items), and someone else's document about ASN1 JER says

there are no restrictions on the kind of white space characters occurring between JSON tokens

[mtfurlan]

Update: I broke the lengths and then thought they were a pre-existing bug.

[mtfurlan]

Because JER is based on XER have references like CANONICAL-JER and BASIC-JER neither of which are things, and we have a jer_encoder_flags_e enum and we pass that around, so if we want to add JER variants for whitespace formatting it probably wouldn't be too much work.

We also have ATS_BASIC_JER which should be ATS_JER unless we do want to do some kind of variant thing for whitespace.

So I propose either A: Only minified javascript, rename ATS_BASIC_JER to ATS_JER, remove the jer encoder flags. B: Do variants of ATS_JER and ATS_JER_PRETTIFIED

[mouse07410]

I'm not sure I fully agree with this... Maybe what you're proposing (ATS_BASIC_JER vs ATS_JER or ATS_JER_PRETTIFIED) is the way to go... Don't know. I'll not rush this.

[mtfurlan]

Yeah, it's a weird choice we shouldn't rush.

I don't like the ATS_JER_PRETTIFIED thing because all the other types are defined by ASN1, and that would be defined by us. Same reason I want to rename ATS_BASIC_JER to ATS_JER, so we match ASN1.

I still kinda think trying to do both whitespace and no whitespace isn't worth the effort here, and if someone wants to have prettified json they should put it through a separate tool like jq.

Also the current formatting isn't great, and while it probably wouldn't be too hard to fix I don't want to.

[mouse07410]

I'm not sure how to proceed here. On the one hand, I hear your arguments. On the other hand, IMHO, a 200KB JSON is an abomination to deal with, regardless of whether it's represented as one long string or pretty-formatted.

Let me hold off on this one.

[mtfurlan]

Would you accept a PR that doesn't modify the formatting but does change ATS_BASIC_JER to ATS_JER?

[mouse07410]

Would you accept a PR that doesn't modify the formatting but does change ATS_BASIC_JER to ATS_JER?

At least I would consider it. ;-)

Seriously, I see no harm in this proposed change. Are these terms defined anywhere, to make sure we don't alter their meanings?

[v0-e]

Some better JSON formatting would be useful. I agree with the use of ATS_JER for the current formatting style. The formatting however still could use some enhancements to make it prettier and easier to read. I'd be willing to add another ATS for a more compact JSON. Maybe ATS_JER_COMPACT or ATS_JER_MINIFIED?

[mouse07410]

Closing in favor of #160.