MTK implementation is currently meant/suitable only for command PMTK001 but there are other messages/commands in MTK

[aldas]

I think MTK implementation is not correct. MTK is command protocol embedded into NMEA sentence. Fields that current MTK struct has are meant for MTK packet "PMTK001" but actually MTK packet can have variable amound of data fields.

See:

• https://www.rhydolabz.com/documents/25/PMTK_A11.pdf
• https://www.sparkfun.com/datasheets/GPS/Modules/PMTK_Protocol.pdf

[icholy]

Not sure how to fix this without a breaking change.

[aldas]

Unless new struct and some ugly if are not added to separate PMTK001 from other MTK command - breaking change is way to go.

In current form I think MTK is quite broken implementation. If breaking change is done this switch

go-nmea/sentence.go

Lines 184 to 187 in e6fa0a7

	switch s.Talker {
	case TypeMTK:
	return newMTK(s)
	}

and this if

go-nmea/sentence.go

Lines 127 to 129 in e6fa0a7

	if strings.HasPrefix(s, "PMTK") {
	return "PMTK", s[4:]
	}

should be also revisited.
At the moment 001 is set to mtk.BaseSentence.Type value. I think Type should still be MTK as s.DataType() is used in switch statements (especially in readme.md examples).

Maybe implementation like that

type MTK struct {
	BaseSentence
	Command MTKBase
}

type MTKBase interface {
	Type() string // last 3 digits of sentence prefix ($PMTK001 = 001)
}

type MTK001 struct {
	Cmd  int64
	Flag int64
}

func (m *MTK001) Type() string {
	return "001"
}

// and other command packets from https://www.sparkfun.com/datasheets/GPS/Modules/PMTK_Protocol.pdf or https://www.rhydolabz.com/documents/25/PMTK_A11.pdf

// newMTK constructor
func newMTK(s BaseSentence) (MTK, error) {
	cmdType := s.Type[3:]
	s.Type = "MTK"
	p := NewParser(s)

	mtk := MTK{
		BaseSentence: s,
		Command:      nil,
	}
	switch cmdType {
	case "001":
		mtk.Command = &MTK001{
			Cmd:  p.Int64(0, "command"),
			Flag: p.Int64(1, "flag"),
		}
	default:
		p.err = fmt.Errorf("unknown MTK command type: %v", cmdType)
	}
	return mtk, p.Err()
}

so you could do switch mtk.Type() { ... } in your own code after parsing the sentence and knowning it is of type MTK

[aldas]

@icholy and @adrianmo: I propose breaking change:

• rename MTK struct to PMTK001
• rename TypeMTK constant to TypePMTK001 with value MTK001
• remove talkerID and prefix related hacks from sentence parsing functions

basically treat this sentence as any other proprietary sentence and if there is someone that needs other MTK protocol sentences in future, these can be implemented as their own structs.

and notify @krasi-georgiev of this breaking change as he is author and probably sole user of MTK001 sentence

This is where struct and constant is used
https://github.com/arribada/LoraTracker/blob/605a0cde5e8cd5cc4f083ef87d779419e2bb8ddd/sender/GPSSender/main.go#L312

from that code is clear that in that case only sentence MTK001 is expected and other MTK sentences are "hand written" in this function https://github.com/arribada/LoraTracker/blob/605a0cde5e8cd5cc4f083ef87d779419e2bb8ddd/sender/GPSSender/main.go#L240

[aldas]

somewhat resolved by #97 MTK is marked deprecated and newer SentenceParser does not add MTK as parser, it has PMTK001 parser. Old global parser still supports MTK