From b594985b2a0a62b97b1b138822a48cfa824a047f Mon Sep 17 00:00:00 2001 From: Chris Keller Date: Wed, 19 May 2021 14:15:01 -0600 Subject: [PATCH] Don't panic or log fatal in the library code (#16) In order to accommodate forward and backward compatibility in the parser, parsing errors cannot be fatal or interrupt the main program. Instead, throw them in an error channel and let the client code decide what to do. Many parsing errors, like missing or extra fields, are not going to be reason to stop listening. --- README.md | 6 ++-- cmd/main.go | 46 +++++++++++++----------- go.mod | 2 +- parser.go | 74 +++++++++++++++++++++----------------- parser_test.go | 6 +++- server.go | 96 ++++++++++++++++++++++++++++++++------------------ 6 files changed, 139 insertions(+), 91 deletions(-) diff --git a/README.md b/README.md index cb49ecc..b1d0cc6 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ -[![PkgGoDev](https://pkg.go.dev/badge/github.com/k0swe/wsjtx-go)](https://pkg.go.dev/github.com/k0swe/wsjtx-go) -[![Go Report Card](https://goreportcard.com/badge/github.com/k0swe/wsjtx-go)](https://goreportcard.com/report/github.com/k0swe/wsjtx-go) -[![Test](https://github.com/k0swe/wsjtx-go/workflows/Test/badge.svg?branch=main)](https://github.com/k0swe/wsjtx-go/actions/workflows/test.yml?query=branch%3Amain) +[![PkgGoDev](https://pkg.go.dev/badge/github.com/k0swe/wsjtx-go/v2)](https://pkg.go.dev/github.com/k0swe/wsjtx-go/v2) +[![Go Report Card](https://goreportcard.com/badge/github.com/k0swe/wsjtx-go/v2)](https://goreportcard.com/report/github.com/k0swe/wsjtx-go/v2) +[![Test](https://github.com/k0swe/wsjtx-go/workflows/Test/badge.svg?branch=v2)](https://github.com/k0swe/wsjtx-go/actions/workflows/test.yml?query=branch%3Av2) # wsjtx-go diff --git a/cmd/main.go b/cmd/main.go index 9c3b2ba..12b3e4f 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -2,8 +2,8 @@ package main import ( "bufio" - "fmt" - "github.com/k0swe/wsjtx-go" + "github.com/k0swe/wsjtx-go/v2" + "log" "os" "reflect" "strings" @@ -11,16 +11,22 @@ import ( // Simple driver binary for wsjtx-go library. func main() { - fmt.Println("Listening for WSJT-X...") + log.Println("Listening for WSJT-X...") + wsjtxServer, err := wsjtx.MakeServer() + if err != nil { + log.Fatalf("%v", err) + } wsjtxChannel := make(chan interface{}, 5) - wsjtxServer := wsjtx.MakeServer() - go wsjtxServer.ListenToWsjtx(wsjtxChannel) + errChannel := make(chan error, 5) + go wsjtxServer.ListenToWsjtx(wsjtxChannel, errChannel) stdinChannel := make(chan string, 5) go stdinCmd(stdinChannel) for { select { + case err := <-errChannel: + log.Printf("error: %v", err) case message := <-wsjtxChannel: handleServerMessage(message) case command := <-stdinChannel: @@ -39,7 +45,7 @@ func stdinCmd(c chan string) { c <- input } if err := scanner.Err(); err != nil { - fmt.Println(err) + log.Println(err) os.Exit(1) } } @@ -49,23 +55,23 @@ func stdinCmd(c chan string) { func handleServerMessage(message interface{}) { switch message.(type) { case wsjtx.HeartbeatMessage: - fmt.Println("Heartbeat:", message) + log.Println("Heartbeat:", message) case wsjtx.StatusMessage: - fmt.Println("Status:", message) + log.Println("Status:", message) case wsjtx.DecodeMessage: - fmt.Println("Decode:", message) + log.Println("Decode:", message) case wsjtx.ClearMessage: - fmt.Println("Clear:", message) + log.Println("Clear:", message) case wsjtx.QsoLoggedMessage: - fmt.Println("QSO Logged:", message) + log.Println("QSO Logged:", message) case wsjtx.CloseMessage: - fmt.Println("Close:", message) + log.Println("Close:", message) case wsjtx.WSPRDecodeMessage: - fmt.Println("WSPR Decode:", message) + log.Println("WSPR Decode:", message) case wsjtx.LoggedAdifMessage: - fmt.Println("Logged Adif:", message) + log.Println("Logged Adif:", message) default: - fmt.Println("Other:", reflect.TypeOf(message), message) + log.Println("Other:", reflect.TypeOf(message), message) } } @@ -75,7 +81,7 @@ func handleCommand(command string, wsjtxServer wsjtx.Server) { switch command { case "hb": - fmt.Println("Sending Heartbeat") + log.Println("Sending Heartbeat") err = wsjtxServer.Heartbeat(wsjtx.HeartbeatMessage{ Id: "wsjtx-go", MaxSchema: 2, @@ -84,19 +90,19 @@ func handleCommand(command string, wsjtxServer wsjtx.Server) { }) case "clear": - fmt.Println("Sending Clear") + log.Println("Sending Clear") err = wsjtxServer.Clear(wsjtx.ClearMessage{Id: "WSJT-X", Window: 2}) case "close": - fmt.Println("Sending Close") + log.Println("Sending Close") err = wsjtxServer.Close(wsjtx.CloseMessage{Id: "WSJT-X"}) case "replay": - fmt.Println("Sending Replay") + log.Println("Sending Replay") err = wsjtxServer.Replay(wsjtx.ReplayMessage{Id: "WSJT-X"}) } if err != nil { - fmt.Println(err) + log.Println(err) } } diff --git a/go.mod b/go.mod index b0eda77..7f829b6 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module github.com/k0swe/wsjtx-go +module github.com/k0swe/wsjtx-go/v2 go 1.14 diff --git a/parser.go b/parser.go index cd1e1d7..64457ce 100644 --- a/parser.go +++ b/parser.go @@ -2,8 +2,8 @@ package wsjtx import ( "encoding/binary" + "fmt" "github.com/leemcloughlin/jdn" - "log" "math" "reflect" "time" @@ -19,62 +19,65 @@ type parser struct { // https://sourceforge.net/p/wsjt/wsjtx/ci/master/tree/Network/NetworkMessage.hpp. This only parses // "Out" or "In/Out" message types and does not include "In" types because they will never be // received by WSJT-X. -func parseMessage(buffer []byte, length int) interface{} { +func parseMessage(buffer []byte, length int) (interface{}, error) { p := parser{buffer: buffer, length: length, cursor: 0} m := p.parseUint32() if m != magic { - // Packet is not speaking the WSJT-X protocol - return nil + return nil, fmt.Errorf("packet is not speaking the WSJT-X protocol") } sch := p.parseUint32() if sch != schema { - log.Println("Got a schema version I wasn't expecting:", sch) + return nil, fmt.Errorf("got a schema version I wasn't expecting: %d", sch) } messageType := p.parseUint32() switch messageType { case heartbeatNum: heartbeat := p.parseHeartbeat() - p.checkParse(heartbeat) - return heartbeat + err := p.checkParse(heartbeat) + return heartbeat, err case statusNum: status := p.parseStatus() - p.checkParse(status) - return status + err := p.checkParse(status) + return status, err case decodeNum: decode := p.parseDecode() - p.checkParse(decode) - return decode + err := p.checkParse(decode) + return decode, err case clearNum: clear := p.parseClear() - p.checkParse(clear) - return clear + err := p.checkParse(clear) + return clear, err case qsoLoggedNum: - qsoLogged := p.parseQsoLogged() - p.checkParse(qsoLogged) - return qsoLogged + qsoLogged, err := p.parseQsoLogged() + if err != nil { + return qsoLogged, err + } + err = p.checkParse(qsoLogged) + return qsoLogged, err case closeNum: closeMsg := p.parseClose() - p.checkParse(closeMsg) - return closeMsg + err := p.checkParse(closeMsg) + return closeMsg, err case wsprDecodeNum: wspr := p.parseWsprDecode() - p.checkParse(wspr) - return wspr + err := p.checkParse(wspr) + return wspr, err case loggedAdifNum: loggedAdif := p.parseLoggedAdif() - p.checkParse(loggedAdif) - return loggedAdif + err := p.checkParse(loggedAdif) + return loggedAdif, err } - return nil + return nil, nil } // Quick sanity check that we parsed all of the message bytes -func (p *parser) checkParse(message interface{}) { +func (p *parser) checkParse(message interface{}) error { if p.cursor != p.length { - log.Fatalf("Parsing WSJT-X %s: There were %d bytes left over\n", + return fmt.Errorf("parsing WSJT-X %s: there were %d bytes left over", reflect.TypeOf(message).Name(), p.length-p.cursor) } + return nil } func (p *parser) parseHeartbeat() HeartbeatMessage { @@ -169,9 +172,13 @@ func (p *parser) parseClear() ClearMessage { } } -func (p *parser) parseQsoLogged() QsoLoggedMessage { +func (p *parser) parseQsoLogged() (QsoLoggedMessage, error) { + var empty QsoLoggedMessage id := p.parseUtf8() - timeOff := p.parseQDateTime() + timeOff, err := p.parseQDateTime() + if err != nil { + return empty, err + } dxCall := p.parseUtf8() dxGrid := p.parseUtf8() txFrequency := p.parseUint64() @@ -181,7 +188,10 @@ func (p *parser) parseQsoLogged() QsoLoggedMessage { txPower := p.parseUtf8() comments := p.parseUtf8() name := p.parseUtf8() - timeOn := p.parseQDateTime() + timeOn, err := p.parseQDateTime() + if err != nil { + return empty, err + } operatorCall := p.parseUtf8() myCall := p.parseUtf8() myGrid := p.parseUtf8() @@ -205,7 +215,7 @@ func (p *parser) parseQsoLogged() QsoLoggedMessage { MyGrid: myGrid, ExchangeSent: exchangeSent, ExchangeReceived: exchangeReceived, - } + }, nil } func (p *parser) parseClose() interface{} { @@ -299,7 +309,7 @@ func (p *parser) parseBool() bool { return value } -func (p *parser) parseQDateTime() time.Time { +func (p *parser) parseQDateTime() (time.Time, error) { julianDay := p.parseUint64() year, month, day := jdn.FromNumber(int(julianDay)) msSinceMidnight := int(p.parseUint32()) @@ -318,7 +328,7 @@ func (p *parser) parseQDateTime() time.Time { // UTC value = time.Date(year, month, day, hour, minute, second, 0, time.UTC) default: - log.Fatalln("WSJT-X parser: Got a timespec I wasn't expecting,", timespec) + return value, fmt.Errorf("got a timespec I wasn't expecting: %d", timespec) } - return value + return value, nil } diff --git a/parser_test.go b/parser_test.go index 4b23ff8..3e35495 100644 --- a/parser_test.go +++ b/parser_test.go @@ -141,7 +141,11 @@ func TestParseMessage(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := parseMessage(tt.args.buffer, tt.args.length); !reflect.DeepEqual(got, tt.want) { + got, err := parseMessage(tt.args.buffer, tt.args.length) + if err != nil { + t.Error(err) + } + if !reflect.DeepEqual(got, tt.want) { t.Errorf("parseMessage() = %v, want %v", got, tt.want) } }) diff --git a/server.go b/server.go index 28c7203..70bff99 100644 --- a/server.go +++ b/server.go @@ -14,125 +14,153 @@ const wsjtxPort = "2237" type Server struct { conn *net.UDPConn remoteAddr *net.UDPAddr + listening bool } -// Create a UDP connection to communicate with WSJT-X. -func MakeServer() Server { +// MakeServer creates a multicast UDP connection to communicate with WSJT-X on the default address +// and port. +func MakeServer() (Server, error) { return MakeMulticastServer(multicastAddr, wsjtxPort) } -func MakeMulticastServer(addrStr string, portStr string) Server { +// MakeMulticastServer creates a multicast UDP connection to communicate with WSJT-X on the given +// address and port. +func MakeMulticastServer(addrStr string, portStr string) (Server, error) { + var empty Server addr, err := net.ResolveUDPAddr("udp", addrStr+":"+portStr) - check(err) + if err != nil { + return empty, err + } conn, err := net.ListenMulticastUDP(addr.Network(), nil, addr) - check(err) - return Server{conn, nil} + if err != nil { + return empty, err + } + return Server{conn, nil, false}, nil } -func MakeUnicastServer(addrStr string, portStr string) Server { +// MakeUnicastServer creates a unicast UDP connection to communicate with WSJT-X on the given +// address and port. +func MakeUnicastServer(addrStr string, portStr string) (Server, error) { + var empty Server addr, err := net.ResolveUDPAddr("udp", addrStr+":"+portStr) - check(err) + if err != nil { + return empty, err + } conn, err := net.ListenUDP(addr.Network(), addr) - check(err) - return Server{conn, nil} + if err != nil { + return empty, err + } + return Server{conn, nil, false}, nil } -// Goroutine to listen for messages from WSJT-X. When heard, the messages are -// parsed and then placed in the given channel. -func (s *Server) ListenToWsjtx(c chan interface{}) { +// ListenToWsjtx listens for messages from WSJT-X. When heard, the messages are parsed and then +// placed in the given message channel. If parsing errors occur, those are reported on the errors +// channel. If a fatal error happens, e.g. the network connection gets closed, the channels are +// closed and the goroutine ends. +func (s *Server) ListenToWsjtx(c chan interface{}, e chan error) { + s.listening = true + defer close(c) + defer close(e) + for { b := make([]byte, bufLen) length, rAddr, err := s.conn.ReadFromUDP(b) - check(err) + if err != nil { + e <- err + s.listening = false + return + } s.remoteAddr = rAddr - message := parseMessage(b, length) + message, err := parseMessage(b, length) + if err != nil { + e <- err + } if message != nil { c <- message } } } -// Send a heartbeat message to WSJT-X. +// Listening returns whether the ListenToWsjtx goroutine is currently running. +func (s *Server) Listening() bool { + return s.listening +} + +// Heartbeat sends a heartbeat message to WSJT-X. func (s *Server) Heartbeat(msg HeartbeatMessage) error { msgBytes, _ := encodeHeartbeat(msg) _, err := s.conn.WriteTo(msgBytes, s.remoteAddr) return err } -// Send a message to WSJT-X to clear the band activity window, the RX frequency -// window, or both. +// Clear sends a message to WSJT-X to clear the band activity window, the RX frequency window, or +// both. func (s *Server) Clear(msg ClearMessage) error { msgBytes, _ := encodeClear(msg) _, err := s.conn.WriteTo(msgBytes, s.remoteAddr) return err } -// Initiate a reply to an earlier decode. The decode message must have started -// with CQ or QRZ. +// Reply initiates a reply to an earlier decode. The decode message must have started with CQ or +// QRZ. func (s *Server) Reply(msg ReplyMessage) error { msgBytes, _ := encodeReply(msg) _, err := s.conn.WriteTo(msgBytes, s.remoteAddr) return err } -// Send a message to WSJT-X to close the program. +// Close sends a message to WSJT-X to close the program. func (s *Server) Close(msg CloseMessage) error { msgBytes, _ := encodeClose(msg) _, err := s.conn.WriteTo(msgBytes, s.remoteAddr) return err } -// Send a message to WSJT-X to replay QSOs in the Band Activity window. +// Replay sends a message to WSJT-X to replay QSOs in the Band Activity window. func (s *Server) Replay(msg ReplayMessage) error { msgBytes, _ := encodeReplay(msg) _, err := s.conn.WriteTo(msgBytes, s.remoteAddr) return err } -// Send a message to WSJT-X to halt transmission. +// HaltTx sends a message to WSJT-X to halt transmission. func (s *Server) HaltTx(msg HaltTxMessage) error { msgBytes, _ := encodeHaltTx(msg) _, err := s.conn.WriteTo(msgBytes, s.remoteAddr) return err } -// Send a message to WSJT-X to set the free text. +// FreeText sends a message to WSJT-X to set the free text of the TX message. func (s *Server) FreeText(msg FreeTextMessage) error { msgBytes, _ := encodeFreeText(msg) _, err := s.conn.WriteTo(msgBytes, s.remoteAddr) return err } -// Send a message to WSJT-X to set this station's Maidenhead grid. +// Location sends a message to WSJT-X to set this station's Maidenhead grid. func (s *Server) Location(msg LocationMessage) error { msgBytes, _ := encodeLocation(msg) _, err := s.conn.WriteTo(msgBytes, s.remoteAddr) return err } -// Send a message to WSJT-X to set callsign highlighting. +// HighlightCallsign sends a message to WSJT-X to set callsign highlighting. func (s *Server) HighlightCallsign(msg HighlightCallsignMessage) error { msgBytes, _ := encodeHighlightCallsign(msg) _, err := s.conn.WriteTo(msgBytes, s.remoteAddr) return err } -// Send a message to WSJT-X to switch to a different pre-defined configuration. +// SwitchConfiguration sends a message to WSJT-X to switch to a different pre-defined configuration. func (s *Server) SwitchConfiguration(msg SwitchConfigurationMessage) error { msgBytes, _ := encodeSwitchConfiguration(msg) _, err := s.conn.WriteTo(msgBytes, s.remoteAddr) return err } -// Send a message to WSJT-X to change various configuration options. +// Configure sends a message to WSJT-X to change various configuration options. func (s *Server) Configure(msg ConfigureMessage) error { msgBytes, _ := encodeConfigure(msg) _, err := s.conn.WriteTo(msgBytes, s.remoteAddr) return err } - -func check(err error) { - if err != nil { - panic(err) - } -}