Add GSensord - not sure how to deal with checksums being optional

[freman]

trafficstars

NB: I don't actually expect you to merge this request, I've broken the handling of missing checksums.

I also think this might be a specific usecase and not a standards type issue.

Basically the Navman MIVUE dashcams add a $GSENSORD sentence that doesn't have checksums.

I did consider refactoring to make types checksum aware so they can complain if they're expecting a checksum or not, but I wasn't sure how you'd like to proceed.

Probably best to just leave it out entirely, or have an unexpected type handler, but then I could just do that with a substring match before parsing.

Sample data:

$GSENSORD,0.060,0.120,-0.180
$GPRMC,073815.000,A,2703.7507,S,15257.3940,E,30.35,36.48,060419,,,A*74
$GPGGA,073815.000,2703.7507,S,15257.3940,E,1,12,0.8,8.4,M,39.6,M,,0000*4B
$GPGSA,A,3,22,31,03,01,193,26,194,18,23,16,14,11,1.6,0.8,1.4*31
$GPGSV,4,1,16,01,44,297,37,03,55,211,28,09,01,263,,11,19,310,22*7C
$GPGSV,4,2,16,14,22,118,27,16,27,032,40,18,33,331,30,22,80,195,37*7D
$GPGSV,4,3,16,23,28,245,32,26,40,065,42,31,42,142,40,32,25,248,*7A
$GPGSV,4,4,16,42,57,343,29,193,78,210,24,194,39,337,36,195,15,346,27*4D
$GSENSORD,-0.120,0.120,-0.250
$GPRMC,073816.000,A,2703.7438,S,15257.3995,E,30.42,35.62,060419,,,A*79
$GPGGA,073816.000,2703.7438,S,15257.3995,E,1,12,0.8,8.4,M,39.6,M,,0000*4D
$GPGSA,A,3,22,31,03,01,193,26,194,18,23,16,14,11,1.6,0.8,1.4*31
$GPGSV,4,1,16,01,44,297,37,03,55,211,27,09,01,263,,11,19,310,21*70
$GPGSV,4,2,16,14,22,118,25,16,27,032,39,18,33,331,31,22,80,195,37*70
$GPGSV,4,3,16,23,28,245,33,26,40,065,43,31,42,142,29,32,25,248,*75
$GPGSV,4,4,16,42,57,343,30,193,78,210,24,194,39,337,36,195,15,346,29*4B

[coveralls]

Coverage remained the same at 100.0% when pulling 0aaf16864ce9548db30cc7e5d94d4be46aff4609 on freman:master into 7572fbeb90aaa0bd3d1dcb674938ffbc0a943123 on adrianmo:master.

[icholy]

It's really annoying that this vendor didn't follow the convention of prefixing proprietary message types with "P" talker id.

[freman]

Yeh that'd be nice.

I thought about adding an api for adding other messages so it didn't require modifying the library for every vendor. But that still runs into the same question of handling the checksum.

[icholy]

@adrianmo I'm ok with this, what do you think?

[icholy]

@adrianmo it's just really annoying that this message type breaks all the conventions/rules. The more I think about it, the more I want to completely special-case it. I need to think about it more.

edit: while we're at it, we could do the same for all "P" prefix messages and the vdo ones.

[freman]

The more I think about it, the more I want to completely special-case it. I need to think about it more.

Yeh I honestly didn't expect this to get merged in any way in it's current format. I figured a refactor around message types and having messages know they should require checksums was in order.

Edit: and if I'm honest I could have just checked if the message was prefixed with $GSENSORD before calling the library and handled it in code outside of the library.

[icholy]

@freman this probably won't land as-is or soon. I'd recommend checking message prefixes in your code for the time being. I'm experimenting with different approaches which give the messages more control over how they are parsed. Once we find a good solution, this will need to be rebased against that. I encourage you to leave this PR open until that happens.

[adrianmo]

@icholy @freman any thoughts on this PR? I'm OK with taking this forward as-is if we address the current and future comments. We can deal with message refactoring later on, in a separate PR.

[icholy]

@adrianmo that sounds fine to me.

[icholy]

With the new SentenceParser being added in #97 , it will be possible to implement a custom parser for GSENSORD. I think that's good enough for now and we don't need to have build-in support for it. Example:

const TypeGSensord = "GSENSORD"

type GSensord struct {
	nmea.BaseSentence
	X float64 // X-axis G value
	Y float64 // Y-axis G value
	Z float64 // Z-axis G valye
}

p := nmea.SentenceParser{
    ParsePrefix: func(prefix string) (string, string, error) {
        if prefix == TypeGSensord {
            return "", prefix, nil
        }
        return nmea.ParsePrefix(prefix)
    },
    CheckCRC: func(s nmea.BaseSentence, fields string) error {
        if s.Type == TypeGSensord {
            return nil
        }
        return nmea.CheckCRC(s, fields)
    },
    CustomParsers: map[string]nmea.ParseFunc{
        TypeGSensord: func(s nmea.BaseSentence) (nmea.Sentence, error) {
            p := nmea.NewParser(s)
            p.AssertType(TypeGSensord)
            m := GSensord{
                BaseSentence: s,
                X:            p.Float64(0, "x-axis g value"),
                Y:            p.Float64(1, "y-axis g value"),
                Z:            p.Float64(2, "z-axis g value"),
            }
            return m, p.Err()
        },
    },
}

m, _ := p.Parse("$GSENSORD,0.060,0.120,-0.180")