More windows fixes (#166)

* Improve pn532 reliability

* Reduce log spam

* Fix allow_file check

pkg/platforms/platforms.go

@@ -112,6 +112,8 @@ type Platform interface {
 	// Launch a system by ID.
 	LaunchSystem(*config.Instance, string) error
 	// Launch a file by path.
+	// TODO: i don't think this needs to exist now launch logic is on the
+	// launcher. better to be one func outside platform
 	LaunchFile(*config.Instance, string) error
 	// Launch a shell command.
 	KeyboardInput(string) error // DEPRECATED

pkg/platforms/steamos/platform.go

@@ -27,6 +27,7 @@ import (
 	"github.com/ZaparooProject/zaparoo-core/pkg/service/tokens"
 	"github.com/ZaparooProject/zaparoo-core/pkg/utils"
 	"github.com/adrg/xdg"
+	"github.com/rs/zerolog/log"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -40,7 +41,6 @@ import (
 	"github.com/ZaparooProject/zaparoo-core/pkg/readers"
 	"github.com/ZaparooProject/zaparoo-core/pkg/readers/file"
 	"github.com/ZaparooProject/zaparoo-core/pkg/readers/simple_serial"
-	"github.com/rs/zerolog/log"
 )
 
 type Platform struct{}
@@ -147,12 +147,18 @@ func (p *Platform) LaunchSystem(_ *config.Instance, _ string) error {
 }
 
 func (p *Platform) LaunchFile(cfg *config.Instance, path string) error {
-	log.Info().Msgf("launching file: %s", path)
 	launchers := utils.PathToLaunchers(cfg, p, path)
 	if len(launchers) == 0 {
 		return errors.New("no launcher found")
 	}
-	return launchers[0].Launch(cfg, path)
+	launcher := launchers[0]
+
+	if launcher.AllowListOnly && !cfg.IsLauncherFileAllowed(path) {
+		return errors.New("file not allowed: " + path)
+	}
+
+	log.Info().Msgf("launching file: %s", path)
+	return launcher.Launch(cfg, path)
 }
 
 func (p *Platform) KeyboardInput(_ string) error {

pkg/platforms/windows/platform.go

@@ -140,16 +140,18 @@ func (p *Platform) LaunchSystem(cfg *config.Instance, id string) error {
 }
 
 func (p *Platform) LaunchFile(cfg *config.Instance, path string) error {
-	log.Info().Msgf("launching file: %s", path)
-
 	launchers := utils.PathToLaunchers(cfg, p, path)
-
 	if len(launchers) == 0 {
 		return errors.New("no launcher found")
 	}
+	launcher := launchers[0]
 
-	// just pick the first one for now
-	return launchers[0].Launch(cfg, path)
+	if launcher.AllowListOnly && !cfg.IsLauncherFileAllowed(path) {
+		return errors.New("file not allowed: " + path)
+	}
+
+	log.Info().Msgf("launching file: %s", path)
+	return launcher.Launch(cfg, path)
 }
 
 func (p *Platform) KeyboardInput(input string) error {

pkg/readers/acr122_pcsc/acr122_pcsc.go

@@ -7,6 +7,7 @@ import (
 	"github.com/ZaparooProject/zaparoo-core/pkg/config"
 	"github.com/ZaparooProject/zaparoo-core/pkg/service/tokens"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/ZaparooProject/zaparoo-core/pkg/readers"
@@ -74,7 +75,8 @@ func (r *Acr122Pcsc) Open(device string, iq chan<- readers.Scan) error {
 			rls, err := ctx.ListReaders()
 			if err != nil {
 				log.Debug().Msgf("error listing pcsc readers: %s", err)
-				continue
+				r.polling = false
+				break
 			}
 
 			if !utils.Contains(rls, r.name) {
@@ -213,6 +215,10 @@ func (r *Acr122Pcsc) Close() error {
 	return nil
 }
 
+// TODO: this is a hack workaround to stop some log spam, probably the Detect
+// functions on readers should actually return an error instead of ""
+var detectErrorOnce sync.Once
+
 func (r *Acr122Pcsc) Detect(connected []string) string {
 	ctx, err := scard.EstablishContext()
 	if err != nil {
@@ -227,10 +233,11 @@ func (r *Acr122Pcsc) Detect(connected []string) string {
 
 	rs, err := ctx.ListReaders()
 	if err != nil {
-		log.Debug().Err(err).Msg("listing pcsc readers")
+		detectErrorOnce.Do(func() {
+			log.Debug().Err(err).Msg("listing pcsc readers")
+		})
 		return ""
 	}
-	// log.Debug().Msgf("pcsc rs: %v", rs)
 
 	acrs := make([]string, 0)
 	for _, r := range rs {

pkg/readers/pn532_uart/ndef.go

@@ -33,46 +33,43 @@ import (
 var NdefEnd = []byte{0xFE}
 var NdefStart = []byte{0x54, 0x02, 0x65, 0x6E}
 
-func ParseRecordText(blocks []byte) (string, error) {
-	startIndex := bytes.Index(blocks, NdefStart)
+var ErrNoNdef = fmt.Errorf("no NDEF record found")
+
+func ParseRecordText(bs []byte) (string, error) {
+	// sometimes there can be some read corruption and multiple copies of the
+	// NDEF header get pulled in. we just flick through until the last one
+	// TODO: is this going to mess up if there are multiple NDEF records?
+	startIndex := bytes.LastIndex(bs, NdefStart)
 	if startIndex == -1 {
-		return "", fmt.Errorf("NDEF start not found: %x", blocks)
+		return "", ErrNoNdef
 	}
 
 	// check if there is another ndef start left, as it can mean we got come
 	// corrupt data at the beginning
-	if len(blocks) > startIndex+8 {
-		nextStart := bytes.Index(blocks[startIndex+4:], NdefStart)
+	if len(bs) > startIndex+8 {
+		nextStart := bytes.Index(bs[startIndex+4:], NdefStart)
 		if nextStart != -1 {
 			startIndex += nextStart
 		}
 	}
 
-	endIndex := bytes.Index(blocks, NdefEnd)
+	endIndex := bytes.Index(bs, NdefEnd)
 	if endIndex == -1 {
-		return "", fmt.Errorf("NDEF end not found: %x", blocks)
+		return "", fmt.Errorf("NDEF end not found: %x", bs)
 	}
 
-	if startIndex >= endIndex || startIndex+4 >= len(blocks) {
-		return "", fmt.Errorf("start index out of bounds: %d, %x", startIndex, blocks)
+	if startIndex >= endIndex || startIndex+4 >= len(bs) {
+		return "", fmt.Errorf("start index out of bounds: %d, %x", startIndex, bs)
 	}
 
-	if endIndex <= startIndex || endIndex >= len(blocks) {
-		return "", fmt.Errorf("end index out of bounds: %d, %x", endIndex, blocks)
+	if endIndex <= startIndex || endIndex >= len(bs) {
+		return "", fmt.Errorf("end index out of bounds: %d, %x", endIndex, bs)
 	}
 
 	log.Debug().Msgf("NDEF start: %d, end: %d", startIndex, endIndex)
-	log.Debug().Msgf("NDEF: %x", blocks[startIndex:endIndex])
-
-	tagText := string(blocks[startIndex+4 : endIndex])
+	log.Debug().Msgf("NDEF: %x", bs[startIndex:endIndex])
 
-	// TODO: why does this happen here but not in libnfc?
-	//cleaned := ""
-	//for _, r := range tagText {
-	//	if r != '\x00' {
-	//		cleaned += string(r)
-	//	}
-	//}
+	tagText := string(bs[startIndex+4 : endIndex])
 
 	return tagText, nil
 }

pkg/readers/pn532_uart/pn532.go

@@ -74,7 +74,7 @@ func waitAck(port serial.Port) ([]byte, error) {
 	// the previous command
 
 	tries := 0
-	maxTries := 64 // bytes to scan through
+	maxTries := 32 // bytes to scan through
 
 	buf := make([]byte, 1)
 	ackBuf := make([]byte, 0)

pkg/readers/pn532_uart/pn532_uart.go

@@ -51,7 +51,7 @@ func connect(name string) (serial.Port, error) {
 		return port, err
 	}
 
-	err = port.SetReadTimeout(100 * time.Millisecond)
+	err = port.SetReadTimeout(50 * time.Millisecond)
 	if err != nil {
 		return port, err
 	}
@@ -157,17 +157,21 @@ func (r *Pn532UartReader) Open(device string, iq chan<- readers.Scan) error {
 				continue
 			}
 
+			ndefRetryMax := 3
+			ndefRetry := 0
+		ndefRetry:
+
 			i := 3
-			retryMax := 3
-			retry := 0
+			blockRetryMax := 3
+			blockRetry := 0
 			data := make([]byte, 0)
 			for {
 				// TODO: this is a random limit i picked, should detect blocks in card
 				if i >= 256 {
 					break
 				}
 
-				if retry >= retryMax {
+				if blockRetry >= blockRetryMax {
 					errCount++
 					break
 				}
@@ -176,7 +180,7 @@ func (r *Pn532UartReader) Open(device string, iq chan<- readers.Scan) error {
 				if errors.Is(err, ErrNoFrameFound) {
 					// sometimes the response just doesn't work, try again
 					log.Warn().Msg("no frame found")
-					retry++
+					blockRetry++
 					continue
 				} else if err != nil {
 					log.Error().Err(err).Msg("failed to run indataexchange")
@@ -190,7 +194,7 @@ func (r *Pn532UartReader) Open(device string, iq chan<- readers.Scan) error {
 					log.Warn().Msgf("unexpected data format: %x", res)
 					// sometimes we receive the result of the last passive
 					// target command, so just try request again a few times
-					retry++
+					blockRetry++
 					continue
 				} else if bytes.Equal(res[2:], make([]byte, 16)) {
 					break
@@ -199,24 +203,22 @@ func (r *Pn532UartReader) Open(device string, iq chan<- readers.Scan) error {
 				data = append(data, res[2:]...)
 				i += 4
 
-				retry = 0
-
-				time.Sleep(1 * time.Millisecond)
+				blockRetry = 0
 			}
 
 			log.Debug().Msgf("record bytes: %s", hex.EncodeToString(data))
 
 			tagText, err := ParseRecordText(data)
-			if err != nil {
-				log.Error().Err(err).Msgf("error parsing NDEF record")
-				// TODO: there should be some distinction between a data
-				// transfer error and a legitimate empty/missing NDEF record
+			if err != nil && ndefRetry < ndefRetryMax {
+				log.Error().Err(err).Msgf("no NDEF found, retrying data exchange")
+				ndefRetry++
+				goto ndefRetry
+			} else if err != nil {
+				log.Error().Err(err).Msgf("no NDEF records")
 				tagText = ""
 			}
 
-			if tagText == "" {
-				log.Warn().Msg("no text NDEF found")
-			} else {
+			if tagText != "" {
 				log.Info().Msgf("decoded text NDEF: %s", tagText)
 			}
 

pkg/service/service.go

@@ -108,15 +108,11 @@ func processTokenQueue(
 	for {
 		select {
 		case pls := <-plq:
-			log.Info().Msgf("processing playlist update: %v", pls)
-
 			activePlaylist := st.GetActivePlaylist()
 
 			if pls == nil {
 				if activePlaylist != nil {
 					log.Info().Msg("clearing active playlist")
-				} else {
-					log.Debug().Msg("no active playlist to clear")
 				}
 				activePlaylist = nil
 				continue