Allow sentence parsing customization (SentenceParser struct) ...

[aldas]

• Allow sentence parsing customization (callbacks for CRC and tagblocks) (SentenceParser struct) - See possibility to make CRC check optional for certain sentences #96
• Add Query sentence,
• Expose all supported types and their parsers as a map (SupportedParsers),
• Add 1.18 into CI flow

BREAKING CHANGE:

• rename MTK sentence to PMTK001, See MTK implementation is currently meant/suitable only for command PMTK001 but there are other messages/commands in MTK  #90

I have added MTK change here as it is deeply tied to sentence parsing internals and has "hacks" to override BaseSentence.TalkerID meaning.

[icholy]

Looks like it's time for a v2.

[aldas]

Just a note Github flow uses go get -u golang.org/x/lint/golint@latest but from Go 1.18 this way of installing commands is removed and go install golang.org/x/lint/golint@latest should be used. But go install is supported only from Go 1.16. Maybe just drop 1.15 and 1.14 from github flow as these versions have been EOL long time now.

[icholy]

Yep, I agree.

[icholy]

This is how I imagine the API:

p := nmea.SentenceParser{
    ParsePrefix: func(prefix string) (string, string, err) {
        if prefix == "GSENSORD" {
            return "", prefix, nil
        }
        return nmea.ParsePrefix(prefix)
    },
    CheckCRC: func(s nmea.BaseSentence, fields string) error {
        if s.Type == "GSENSORD" {
            return nil
        }
        return nmea.CheckCRC(s, fields)
    },
}


p.Register("GSENSORD", func(s nmea.BaseSentence) (nmea.Sentence, error) {
    // ...
})

The zero value nmea.SentenceParser is valid.

var p nmea.SentenceParser
_, _ = p.Parse("...")

[aldas]

At the moment I would suggest

	p := nmea.NewSentenceParserWithConfig(nmea.SentenceParserConfig{
		CustomParsers: map[string]nmea.ParserFunc{
			"XYZ": func(s nmea.BaseSentence) (nmea.Sentence, error) {
				// This example uses the package builtin parsing helpers
				// you can implement your own parsing logic also
				p := nmea.NewParser(s)
				return XYZType{
					BaseSentence: s,
					Time:         p.Time(0, "time"),
					Label:        p.String(1, "label"),
					Counter:      p.Int64(2, "counter"),
					Value:        p.Float64(3, "value"),
				}, p.Err()
			},
		},
	})
	s, err := p.Parse(`$00XYZ,220516,A,23,5133.82,W*42`)

p := nmea.NewSentenceParserWithConfig(nmea.SentenceParserConfig{ is way longer (at least first part) but by creating instance with function you no not need to ever argue with anyone, should it be co-routine safe or not. This aligns with global sentence parser as it seems also be co-routine safe in regards of custom parsers. This PR introduces these new additional configurable callbacks that could also be potentially be racy.
My experience with opensource libraries is the if there is a way - then someone will shoot them in the foot and very few read comments/warning on methods/functions whether something is or is not co-routine safe.

// SentenceParser provides configurable functionality to parse raw input into Sentence
type SentenceParser struct {
	// parserLock exists because we want to preserve defaultSentenceParser backwards compatibility so global
	// RegisterParser and MustRegisterParser functions can mutate customParsers map. SentenceParser instance itself
	// does not need locks
	parserLock *sync.RWMutex
	config     SentenceParserConfig
}

// NewSentenceParser creates new default instance of SentenceParser
func NewSentenceParser() *SentenceParser {
	return NewSentenceParserWithConfig(SentenceParserConfig{})
}

// NewSentenceParserWithConfig creates new instance of SentenceParser with given config
func NewSentenceParserWithConfig(c SentenceParserConfig) *SentenceParser {
	if c.ParseAddress == nil {
		c.ParseAddress = DefaultParseAddress
	}
	if c.CheckCRC == nil {
		c.CheckCRC = DefaultCheckCRC
	}
	cp := map[string]ParserFunc{}
	for sType, pFunc := range c.CustomParsers {
		cp[sType] = pFunc
	}

	return &SentenceParser{
		parserLock: &sync.RWMutex{},
		config: SentenceParserConfig{
			CustomParsers: cp,
			ParseAddress:  c.ParseAddress,
			CheckCRC:      c.CheckCRC,
			OnTagBlock:    c.OnTagBlock,
		},
	}
}

[icholy]

p := nmea.NewSentenceParserWithConfig(nmea.SentenceParserConfig{ is way longer (at least first part) but by creating instance with function you no not need to ever argue with anyone, should it be co-routine safe or not. This aligns with global sentence parser as it seems also be co-routine safe in regards of custom parsers.

The top level functions are thread safe because it's likely that code in different packages/modules use them without being aware of each-other. If a package is creating its own nmea.SentenceParser then it can guarantee that no-one else is using it. In general, types are not thread safe by default because it adds unnecessary overhead and the user can lock around it if they need to.

This PR introduces these new additional configurable callbacks that could also be potentially be racy.

How would the suggested lock prevent races in the user-provider functions?

My experience with opensource libraries is the if there is a way - then someone will shoot them in the foot and very few read comments/warning on methods/functions whether something is or is not co-routine safe.

Then let them shoot themselves in the foot. Maybe they'll learn to read the docs.

[aldas]

alright, this should be better now. Changed mutex parts + SenteceParser is plain struct with public fields. MTK changes are resolved as - global Parse work with old MTK parser and when SentenceParser instance is created - it does not contain old MTK parser and works with newer PMTK001 Ack parser.

[aldas]

If it comes to merging some day - please use Squash or allow me to squash commits

[icholy]

gofmt the code and it's good to go.

[icholy]

@aldas feel free to merge if you're ready. @adrianmo FYI