From c712540558f5f76a98209f34ef8677461681fb1e Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Fri, 3 Apr 2026 18:13:01 +0800 Subject: [PATCH] fix(readers): remove legacy pn532uart driver Delete the standalone pn532uart package and remove it from Windows SupportedReaders. The legacy driver had a permanent serial port blocklist that never cleared entries, causing intermittent detection failures when multiple PN532 readers were connected (issue #602). The modern pn532 driver handles UART detection with proper ModTime-based cache eviction and already accepts the legacy driver IDs for backwards compatibility. Closes #602 --- docs/ARCHITECTURE.md | 2 +- pkg/platforms/windows/platform.go | 2 - pkg/readers/pn532uart/main_test.go | 30 -- pkg/readers/pn532uart/pn532.go | 436 ---------------- pkg/readers/pn532uart/pn532uart.go | 465 ------------------ .../pn532uart/pn532uart_integration_test.go | 422 ---------------- pkg/readers/pn532uart/pn532uart_test.go | 140 ------ 7 files changed, 1 insertion(+), 1496 deletions(-) delete mode 100644 pkg/readers/pn532uart/main_test.go delete mode 100644 pkg/readers/pn532uart/pn532.go delete mode 100644 pkg/readers/pn532uart/pn532uart.go delete mode 100644 pkg/readers/pn532uart/pn532uart_integration_test.go delete mode 100644 pkg/readers/pn532uart/pn532uart_test.go diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index e28d21ab1..1193041d4 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -45,4 +45,4 @@ Reference material for Zaparoo Core's architecture, APIs, and subsystems. For de ## Reader Auto-Detection -11 reader types: acr122pcsc, externaldrive, file, libnfc, mqtt, opticaldrive, pn532, pn532uart, rs232barcode, simpleserial, tty2oled +10 reader types: acr122pcsc, externaldrive, file, libnfc, mqtt, opticaldrive, pn532, rs232barcode, simpleserial, tty2oled diff --git a/pkg/platforms/windows/platform.go b/pkg/platforms/windows/platform.go index 13f437dd5..7cdfd7ff1 100644 --- a/pkg/platforms/windows/platform.go +++ b/pkg/platforms/windows/platform.go @@ -52,7 +52,6 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers/file" "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers/mqtt" "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers/pn532" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers/pn532uart" "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers/rs232barcode" "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers/simpleserial" "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers/tty2oled" @@ -82,7 +81,6 @@ func (*Platform) ID() string { func (p *Platform) SupportedReaders(cfg *config.Instance) []readers.Reader { allReaders := []readers.Reader{ pn532.NewReader(cfg), - pn532uart.NewReader(cfg), file.NewReader(cfg), simpleserial.NewReader(cfg), rs232barcode.NewReader(cfg), diff --git a/pkg/readers/pn532uart/main_test.go b/pkg/readers/pn532uart/main_test.go deleted file mode 100644 index 53c98d5dc..000000000 --- a/pkg/readers/pn532uart/main_test.go +++ /dev/null @@ -1,30 +0,0 @@ -// Zaparoo Core -// Copyright (c) 2026 The Zaparoo Project Contributors. -// SPDX-License-Identifier: GPL-3.0-or-later -// -// This file is part of Zaparoo Core. -// -// Zaparoo Core is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// Zaparoo Core is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with Zaparoo Core. If not, see . - -package pn532uart - -import ( - "testing" - - "go.uber.org/goleak" -) - -func TestMain(m *testing.M) { - goleak.VerifyTestMain(m) -} diff --git a/pkg/readers/pn532uart/pn532.go b/pkg/readers/pn532uart/pn532.go deleted file mode 100644 index 729a7cebc..000000000 --- a/pkg/readers/pn532uart/pn532.go +++ /dev/null @@ -1,436 +0,0 @@ -// Zaparoo Core -// Copyright (c) 2026 The Zaparoo Project Contributors. -// SPDX-License-Identifier: GPL-3.0-or-later -// -// This file is part of Zaparoo Core. -// -// Zaparoo Core is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// Zaparoo Core is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with Zaparoo Core. If not, see . - -package pn532uart - -import ( - "bytes" - "encoding/hex" - "errors" - "fmt" - "strconv" - "time" - - "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/tokens" - "github.com/rs/zerolog/log" - "go.bug.st/serial" -) - -const ( - cmdSamConfiguration = 0x14 - cmdGetFirmwareVersion = 0x02 - cmdGetGeneralStatus = 0x04 - cmdInListPassiveTarget = 0x4A - cmdInDataExchange = 0x40 - hostToPn532 = 0xD4 - pn532ToHost = 0xD5 - pn532Ready = 0x01 -) - -var ( - ackFrame = []byte{0x00, 0x00, 0xFF, 0x00, 0xFF, 0x00} - nackFrame = []byte{0x00, 0x00, 0xFF, 0xFF, 0x00, 0x00} - ErrAckTimeout = errors.New("timeout waiting for ACK") - ErrNoFrameFound = errors.New("no frame found") -) - -func wakeUp(port serial.Port) error { - // over uart, pn532 must be (to be safe) "woken up" by sending a 0x55 - // dummy byte and then waiting for some amount of time - - n, err := port.Write([]byte{ - 0x55, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - }) - if err != nil { - return fmt.Errorf("failed to write wakeup bytes: %w", err) - } else if n != 16 { - return errors.New("wakeup write error, not all bytes written") - } - - err = port.Drain() - if err != nil { - return fmt.Errorf("failed to drain port: %w", err) - } - - return nil -} - -func sendAck(port serial.Port) error { - // tells the PN532 the command was received ok! (optional) - // can also be used to immediately cancel the current processing command - - n, err := port.Write(ackFrame) - if err != nil { - return fmt.Errorf("failed to write ACK frame: %w", err) - } else if n != len(ackFrame) { - return errors.New("ack write error, not all bytes written") - } - - if err := port.Drain(); err != nil { - return fmt.Errorf("failed to drain port after ACK: %w", err) - } - return nil -} - -// Block and wait to receive an ACK frame on the serial port, returning any -// extra data that was received before the ACK frame. Data before the ACK frame -// is not to spec, but is an odd bug happening on Windows. -func waitAck(port serial.Port) ([]byte, error) { - // pn532 will send this sequence to acknowledge it received - // the previous command - - tries := 0 - maxTries := 32 // bytes to scan through - - buf := make([]byte, 1) - ackBuf := make([]byte, 0) - preAck := make([]byte, 0) - - for { - if tries >= maxTries { - return preAck, ErrAckTimeout - } - - n, err := port.Read(buf) - if err != nil { - return preAck, fmt.Errorf("failed to read from port while waiting for ACK: %w", err) - } else if n == 0 { - tries++ - continue - } - - ackBuf = append(ackBuf, buf[0]) - if len(ackBuf) < 6 { - continue - } - - if bytes.Equal(ackBuf, ackFrame) { - return preAck, nil - } - preAck = append(preAck, ackBuf[0]) - ackBuf = ackBuf[1:] - tries++ - continue - } -} - -func sendNack(port serial.Port) error { - // tells the PN532 there was a problem and to resend previous data - n, err := port.Write(nackFrame) - if err != nil { - return fmt.Errorf("failed to write NACK frame: %w", err) - } else if n != len(nackFrame) { - return errors.New("nack write error, not all bytes written") - } - - if err := port.Drain(); err != nil { - return fmt.Errorf("failed to drain port after NACK: %w", err) - } - return nil -} - -func sendFrame(port serial.Port, cmd byte, args []byte) ([]byte, error) { - // create frame - frm := []byte{0x00, 0x00, 0xFF} // preamble and start code - - data := make([]byte, 0, 2+len(args)) - data = append(data, hostToPn532, cmd) - data = append(data, args...) - - if len(data) > 255 { - // TODO: extended frames are not implemented - return []byte{}, errors.New("data too big for frame") - } - - dlen := byte(len(data)) //nolint:gosec // G115: int->byte after len(data) > 255 guard above - frm = append(frm, dlen, ^dlen+1) // length and length checksum - - checksum := byte(0) - - for _, b := range data { - frm = append(frm, b) - checksum += b - } - - frm = append(frm, ^checksum+1, 0x00) // data checksum and postamble - - // write frame - err := wakeUp(port) - if err != nil { - return []byte{}, err - } - - n, err := port.Write(frm) - if err != nil { - return []byte{}, fmt.Errorf("failed to write frame: %w", err) - } else if n != len(frm) { - return []byte{}, errors.New("write error, not all bytes written") - } - - err = port.Drain() - if err != nil { - return []byte{}, fmt.Errorf("failed to drain port after frame write: %w", err) - } - - return waitAck(port) -} - -// Read a single frame from the serial port, returning the data part of the -// frame. Optionally accepts data to prepend to the read buffer and -// treat as part of the potential frame. -func receiveFrame(port serial.Port, pre []byte) ([]byte, error) { - tries := 0 - maxTries := 3 - -retry: - buf := make([]byte, 255+7) - if tries == 0 { - // prepend any leftover response from a skipped ACK - newBuf := make([]byte, len(pre)+len(buf)) - copy(newBuf, pre) - copy(newBuf[len(pre):], buf) - buf = newBuf - } - - _, err := port.Read(buf) - if err != nil { - return []byte{}, fmt.Errorf("failed to read response frame: %w", err) - } - - // find middle of packet code (0x00 0xff) and skip preamble - off := 0 - for ; off < len(buf); off++ { - if buf[off] == 0xFF { - break - } - } - if off == len(buf) { - return []byte{}, ErrNoFrameFound - } - - // check frame length value and checksum (LEN) - off++ - frameLen := int(buf[off]) - if ((frameLen + int(buf[off+1])) & 0xFF) != 0 { - if tries < maxTries { - tries++ - err := sendNack(port) - if err != nil { - return []byte{}, err - } - log.Debug().Msg("invalid frame length, sending NACK") - goto retry - } - return []byte{}, errors.New("invalid frame length") - } - - // check frame checksum against data (LCS) - chk := byte(0) - for _, b := range buf[off+2 : off+2+frameLen+1] { - chk += b - } - if chk != 0 { - if tries < maxTries { - tries++ - err := sendNack(port) - if err != nil { - return []byte{}, err - } - log.Debug().Msg("invalid frame checksum, sending NACK") - goto retry - } - return []byte{}, errors.New("invalid frame checksum") - } - - // check tfi - off += 2 - if buf[off] != pn532ToHost { - if tries < maxTries { - tries++ - err := sendNack(port) - if err != nil { - return []byte{}, err - } - log.Debug().Msg("invalid TFI, sending NACK") - goto retry - } - got := strconv.FormatUint(uint64(buf[off]), 16) - return []byte{}, errors.New("invalid TFI, expected PN532 to host, got: " + got) - } - - // get frame data - off++ - - // return data part of frame - data := make([]byte, frameLen-1) - copy(data, buf[off:off+frameLen-1]) - - return data, nil -} - -func callCommand( - port serial.Port, - cmd byte, - data []byte, -) ([]byte, error) { - ackData, err := sendFrame(port, cmd, data) - if err != nil { - return []byte{}, err - } - - if len(ackData) > 0 { - log.Debug().Msgf("pre ack data: %x", ackData) - } - - time.Sleep(6 * time.Millisecond) - - res, err := receiveFrame(port, ackData) - if err != nil { - return []byte{}, err - } - - err = sendAck(port) - if err != nil { - return []byte{}, err - } - - return res, nil -} - -func SamConfiguration(port serial.Port) error { - log.Debug().Msg("running sam configuration") - // sets pn532 to "normal" mode - res, err := callCommand(port, cmdSamConfiguration, []byte{0x01, 0x14, 0x01}) - if err != nil { - return err - } else if len(res) != 1 || res[0] != 0x15 { - return errors.New("unexpected sam configuration response") - } - - return nil -} - -type FirmwareVersion struct { - Version string - SupportIso14443a bool - SupportIso14443b bool - SupportIso18092 bool -} - -func GetFirmwareVersion(port serial.Port) (FirmwareVersion, error) { - log.Debug().Msg("running getfirmwareversion") - res, err := callCommand(port, cmdGetFirmwareVersion, []byte{}) - if err != nil { - return FirmwareVersion{}, err - } else if len(res) != 5 || res[0] != 0x03 { - return FirmwareVersion{}, errors.New("unexpected firmware version response") - } - - if res[1] != 0x32 { - return FirmwareVersion{}, fmt.Errorf("unexpected IC: %x", res[1]) - } - - fv := FirmwareVersion{ - Version: fmt.Sprintf("%d.%d", res[2], res[3]), - SupportIso14443a: res[4]&0x01 == 0x01, - SupportIso14443b: res[4]&0x02 == 0x02, - SupportIso18092: res[4]&0x04 == 0x04, - } - - return fv, nil -} - -type GeneralStatus struct { - LastError byte - FieldPresent bool -} - -func GetGeneralStatus(port serial.Port) (GeneralStatus, error) { - log.Debug().Msg("running getgeneralstatus") - res, err := callCommand(port, cmdGetGeneralStatus, []byte{}) - if err != nil { - return GeneralStatus{}, err - } else if len(res) < 4 || res[0] != 0x05 { - return GeneralStatus{}, errors.New("unexpected general status response") - } - - gs := GeneralStatus{ - LastError: res[1], - FieldPresent: res[2] == 0x01, - } - - return gs, nil -} - -type Target struct { - Type string - UID string - UIDBytes []byte -} - -func InListPassiveTarget(port serial.Port) (*Target, error) { - res, err := callCommand(port, cmdInListPassiveTarget, []byte{0x01, 0x00}) - switch { - case errors.Is(err, ErrNoFrameFound): - // no tag detected - return nil, nil //nolint:nilnil // nil response means no target detected - case err != nil: - return nil, err - case len(res) < 2 || res[0] != 0x4B: - return nil, errors.New("unexpected passive target response") - case res[1] != 0x01: - // no tag detected - return nil, nil //nolint:nilnil // nil response means no target detected - } - - uidLen := res[6] - if uidLen == 0 { - return nil, errors.New("invalid uid length") - } - - uid := res[7 : 7+uidLen] - uidStr := hex.EncodeToString(uid) - - tagType := "" - if bytes.Equal(res[3:6], []byte{0x00, 0x04, 0x08}) { - tagType = tokens.TypeMifare - } else if bytes.Equal(res[3:6], []byte{0x00, 0x44, 0x00}) { - tagType = tokens.TypeNTAG - } - - return &Target{ - Type: tagType, - UID: uidStr, - UIDBytes: uid, - }, nil -} - -func InDataExchange(port serial.Port, data []byte) ([]byte, error) { - log.Debug().Msg("running indataexchange") - res, err := callCommand(port, cmdInDataExchange, append([]byte{0x01}, data...)) - if err != nil { - return []byte{}, err - } else if len(res) < 2 { - return []byte{}, errors.New("unexpected data exchange response") - } - - return res, nil -} diff --git a/pkg/readers/pn532uart/pn532uart.go b/pkg/readers/pn532uart/pn532uart.go deleted file mode 100644 index c27ced461..000000000 --- a/pkg/readers/pn532uart/pn532uart.go +++ /dev/null @@ -1,465 +0,0 @@ -// Zaparoo Core -// Copyright (c) 2026 The Zaparoo Project Contributors. -// SPDX-License-Identifier: GPL-3.0-or-later -// -// This file is part of Zaparoo Core. -// -// Zaparoo Core is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// Zaparoo Core is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with Zaparoo Core. If not, see . - -package pn532uart - -import ( - "bytes" - "encoding/hex" - "errors" - "fmt" - "os" - "path/filepath" - "runtime" - "strings" - "time" - - "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/config" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/syncutil" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers/shared/ndef" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/tokens" - "github.com/rs/zerolog/log" - "go.bug.st/serial" -) - -// SerialPort defines the interface for serial port operations (for mocking in tests). -type SerialPort interface { - Close() error -} - -// ConnectFunc creates and initializes a PN532 UART connection. -type ConnectFunc func(name string) (SerialPort, error) - -// PN532Commander handles PN532 protocol commands. -type PN532Commander interface { - InListPassiveTarget(port SerialPort) (*Target, error) - InDataExchange(port SerialPort, data []byte) ([]byte, error) -} - -// DefaultPN532Commander uses the real PN532 command functions. -type DefaultPN532Commander struct{} - -func (DefaultPN532Commander) InListPassiveTarget(port SerialPort) (*Target, error) { - // Cast to serial.Port for the actual command - if sp, ok := port.(serial.Port); ok { - return InListPassiveTarget(sp) - } - return nil, errors.New("invalid port type") -} - -func (DefaultPN532Commander) InDataExchange(port SerialPort, data []byte) ([]byte, error) { - // Cast to serial.Port for the actual command - if sp, ok := port.(serial.Port); ok { - return InDataExchange(sp, data) - } - return nil, errors.New("invalid port type") -} - -// DefaultConnectFunc wraps the package-level connect function. -func DefaultConnectFunc(name string) (SerialPort, error) { - return connect(name) -} - -type PN532UARTReader struct { - port SerialPort - commander PN532Commander - cfg *config.Instance - lastToken *tokens.Token - connectFn ConnectFunc - device config.ReadersConnect - name string - polling bool - mu syncutil.RWMutex // protects polling -} - -func NewReader(cfg *config.Instance) *PN532UARTReader { - return &PN532UARTReader{ - cfg: cfg, - connectFn: DefaultConnectFunc, - commander: DefaultPN532Commander{}, - } -} - -func (*PN532UARTReader) Metadata() readers.DriverMetadata { - return readers.DriverMetadata{ - ID: "legacypn532uart", - DefaultEnabled: true, - DefaultAutoDetect: false, - Description: "Legacy PN532 UART reader", - } -} - -func (*PN532UARTReader) IDs() []string { - return []string{"legacypn532uart", "legacy_pn532_uart"} -} - -func connect(name string) (serial.Port, error) { - log.Debug().Msgf("connecting to %s", name) - port, err := serial.Open(name, &serial.Mode{ - BaudRate: 115200, - DataBits: 8, - Parity: serial.NoParity, - StopBits: serial.OneStopBit, - }) - if err != nil { - return port, fmt.Errorf("failed to open serial port: %w", err) - } - - err = port.SetReadTimeout(50 * time.Millisecond) - if err != nil { - return port, fmt.Errorf("failed to set read timeout: %w", err) - } - - err = SamConfiguration(port) - if err != nil { - return port, err - } - - fv, err := GetFirmwareVersion(port) - if err != nil { - return port, err - } - log.Debug().Msgf("firmware version: %v", fv) - - return port, nil -} - -func (r *PN532UARTReader) Open(device config.ReadersConnect, iq chan<- readers.Scan, _ readers.OpenOpts) error { - if !helpers.Contains(r.IDs(), device.Driver) { - return errors.New("invalid reader id: " + device.Driver) - } - - name := device.Path - - if runtime.GOOS != "windows" { - if _, err := os.Stat(name); err != nil { - return fmt.Errorf("device path does not exist: %w", err) - } - } - - port, err := r.connectFn(name) - if err != nil { - if port != nil { - _ = port.Close() - } - return err - } - - r.port = port - r.device = device - r.name = name - r.mu.Lock() - r.polling = true - r.mu.Unlock() - - go func() { - errCount := 0 - maxErrors := 5 - zeroScans := 0 - maxZeroScans := 3 - - for { - r.mu.RLock() - polling := r.polling - r.mu.RUnlock() - if !polling { - break - } - if errCount >= maxErrors { - log.Warn().Msg("too many errors, exiting") - - log.Warn().Msg("reader error, sending error signal") - iq <- readers.Scan{ - Source: tokens.SourceReader, - Token: nil, - ReaderError: true, - } - r.lastToken = nil - - err := r.Close() - if err != nil { - log.Warn().Err(err).Msg("failed to close serial port") - } - break - } - - time.Sleep(250 * time.Millisecond) - - tgt, err := r.commander.InListPassiveTarget(r.port) - if err != nil { - log.Warn().Err(err).Msg("failed to read passive target") - errCount++ - continue - } else if tgt == nil { - zeroScans++ - - // token was removed - if zeroScans == maxZeroScans && r.lastToken != nil { - if r.lastToken != nil { - iq <- readers.Scan{ - Source: tokens.SourceReader, - Token: nil, - } - r.lastToken = nil - } - } - - continue - } - - errCount = 0 - zeroScans = 0 - - if r.lastToken != nil && r.lastToken.UID == tgt.UID { - // same token - continue - } - - if tgt.Type == tokens.TypeMifare { - log.Debug().Msg("mifare not supported, skipping") - continue - } - - ndefRetryMax := 3 - ndefRetry := 0 - ndefRetry: - - i := 3 - blockRetryMax := 3 - blockRetry := 0 - data := make([]byte, 0) - readLoop: - for i < 256 { - // TODO: this is a random limit i picked, should detect blocks in card - - if blockRetry >= blockRetryMax { - errCount++ - break readLoop - } - - res, exchangeErr := r.commander.InDataExchange(r.port, []byte{0x30, byte(i)}) - switch { - case errors.Is(exchangeErr, ErrNoFrameFound): - // sometimes the response just doesn't work, try again - log.Warn().Msg("no frame found") - blockRetry++ - continue readLoop - case exchangeErr != nil: - log.Warn().Err(exchangeErr).Msg("failed to run indataexchange") - errCount++ - break readLoop - case len(res) < 2: - log.Warn().Msg("unexpected data response length") - errCount++ - break readLoop - case res[0] != 0x41 || res[1] != 0x00: - 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 - blockRetry++ - continue readLoop - case bytes.Equal(res[2:], make([]byte, 16)): - break readLoop - } - - data = append(data, res[2:]...) - i += 4 - - blockRetry = 0 - } - - log.Debug().Msgf("record bytes: %s", hex.EncodeToString(data)) - - tagText, err := ndef.ParseToText(data) - if err != nil && ndefRetry < ndefRetryMax { - log.Warn().Err(err).Msgf("no NDEF found, retrying data exchange") - ndefRetry++ - goto ndefRetry - } else if err != nil { - log.Warn().Err(err).Msgf("no NDEF records") - tagText = "" - } - - if tagText != "" { - log.Info().Msgf("decoded text NDEF: %s", tagText) - } - - token := &tokens.Token{ - Type: tgt.Type, - UID: tgt.UID, - Text: tagText, - Data: hex.EncodeToString(data), - ScanTime: time.Now(), - Source: tokens.SourceReader, - ReaderID: r.ReaderID(), - } - - if !helpers.TokensEqual(token, r.lastToken) { - iq <- readers.Scan{ - Source: tokens.SourceReader, - Token: token, - } - } - - r.lastToken = token - } - }() - - return nil -} - -func (r *PN532UARTReader) Close() error { - r.mu.Lock() - r.polling = false - r.mu.Unlock() - if r.port != nil { - err := r.port.Close() - if err != nil { - return fmt.Errorf("failed to close serial port: %w", err) - } - } - return nil -} - -// keep track of serial devices that had failed opens -var ( - serialCacheMu = &syncutil.RWMutex{} - serialBlockList []string -) - -func (*PN532UARTReader) Detect(connected []string) string { - ports, err := helpers.GetSerialDeviceList() - if err != nil { - log.Warn().Err(err).Msg("failed to get serial ports") - } - - for _, name := range ports { - device := "legacypn532uart:" + name - - // ignore if device is in block list - serialCacheMu.RLock() - if helpers.Contains(serialBlockList, name) { - serialCacheMu.RUnlock() - continue - } - serialCacheMu.RUnlock() - - // ignore if exact same device and reader are connected - if helpers.Contains(connected, device) { - continue - } - - if runtime.GOOS != "windows" { - // resolve device symlink if necessary - realPath := "" - symPath, err := os.Readlink(name) - if err == nil { - parent := filepath.Dir(name) - abs, err := filepath.Abs(filepath.Join(parent, symPath)) - if err == nil { - realPath = abs - } - } - - // ignore if same resolved device and reader connected - if realPath != "" && helpers.Contains(connected, realPath) { - continue - } - - // ignore if different resolved device and reader connected - if realPath != "" && strings.HasSuffix(realPath, ":"+realPath) { - continue - } - } - - // ignore if different reader already connected - match := false - for _, connDev := range connected { - if strings.HasSuffix(connDev, ":"+name) { - match = true - break - } - } - if match { - continue - } - - // try to open the device - port, err := connect(name) - if port != nil { - closeErr := port.Close() - if closeErr != nil { - log.Warn().Err(closeErr).Msg("failed to close serial port") - } - } - - if err != nil { - log.Trace().Err(err).Msgf("failed to open detected serial port, blocklisting: %s", name) - serialCacheMu.Lock() - serialBlockList = append(serialBlockList, name) - serialCacheMu.Unlock() - continue - } - - return device - } - - return "" -} - -func (r *PN532UARTReader) Path() string { - return r.device.Path -} - -func (r *PN532UARTReader) ReaderID() string { - stablePath := helpers.GetUSBTopologyPath(r.device.Path) - if stablePath == "" { - stablePath = r.device.ConnectionString() - } - return readers.GenerateReaderID(r.Metadata().ID, stablePath) -} - -func (r *PN532UARTReader) Connected() bool { - r.mu.RLock() - defer r.mu.RUnlock() - return r.polling && r.port != nil -} - -func (r *PN532UARTReader) Info() string { - return "PN532 UART (" + r.name + ")" -} - -func (*PN532UARTReader) Write(_ string) (*tokens.Token, error) { - return nil, errors.New("writing not supported on this reader") -} - -func (*PN532UARTReader) CancelWrite() { - // no-op, writing not supported -} - -func (*PN532UARTReader) Capabilities() []readers.Capability { - return []readers.Capability{readers.CapabilityRemovable} -} - -func (*PN532UARTReader) OnMediaChange(*models.ActiveMedia) error { - return nil -} diff --git a/pkg/readers/pn532uart/pn532uart_integration_test.go b/pkg/readers/pn532uart/pn532uart_integration_test.go deleted file mode 100644 index 1dba787f6..000000000 --- a/pkg/readers/pn532uart/pn532uart_integration_test.go +++ /dev/null @@ -1,422 +0,0 @@ -// Zaparoo Core -// Copyright (c) 2026 The Zaparoo Project Contributors. -// SPDX-License-Identifier: GPL-3.0-or-later -// -// This file is part of Zaparoo Core. -// -// Zaparoo Core is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// Zaparoo Core is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with Zaparoo Core. If not, see . - -package pn532uart - -import ( - "testing" - "time" - - "github.com/ZaparooProject/zaparoo-core/v2/pkg/config" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/syncutil" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers/testutils" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/tokens" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// mockSerialPort is a mock implementation of SerialPort for testing. -type mockSerialPort struct { - closed bool - mu syncutil.RWMutex // protects closed -} - -func (m *mockSerialPort) Close() error { - m.mu.Lock() - m.closed = true - m.mu.Unlock() - return nil -} - -func (m *mockSerialPort) IsClosed() bool { - m.mu.RLock() - defer m.mu.RUnlock() - return m.closed -} - -// mockPN532Commander is a mock implementation of PN532Commander for testing. -type mockPN532Commander struct { - listPassiveTargetFunc func(port SerialPort) (*Target, error) - dataExchangeFunc func(port SerialPort, data []byte) ([]byte, error) -} - -func (m *mockPN532Commander) InListPassiveTarget(port SerialPort) (*Target, error) { - if m.listPassiveTargetFunc != nil { - return m.listPassiveTargetFunc(port) - } - return nil, nil //nolint:nilnil // nil target means no tag detected, which is valid behavior -} - -func (m *mockPN532Commander) InDataExchange(port SerialPort, data []byte) ([]byte, error) { - if m.dataExchangeFunc != nil { - return m.dataExchangeFunc(port, data) - } - return []byte{}, nil -} - -// TestOpen_ErrorCountExceedsMaxWithActiveToken tests the fix for issue #326. -// When error count exceeds maxErrors (5) and there's an active token, -// ReaderError should be set to true to prevent triggering on_remove hooks. -func TestOpen_ErrorCountExceedsMaxWithActiveToken(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test in short mode") - } - t.Parallel() - - cfg := &config.Instance{} - reader := NewReader(cfg) - scanQueue := testutils.CreateTestScanChannel(t) - - mockPort := &mockSerialPort{} - callCount := 0 - - // Connect function returns mock port - reader.connectFn = func(_ string) (SerialPort, error) { - return mockPort, nil - } - - // First call succeeds with a token, subsequent calls fail to trigger error count - reader.commander = &mockPN532Commander{ - listPassiveTargetFunc: func(_ SerialPort) (*Target, error) { - callCount++ - if callCount == 1 { - // First call: successful token detection - return &Target{ - Type: tokens.TypeNTAG, - UID: "test-uid-123", - }, nil - } - // Subsequent calls: errors to increment errCount - return nil, assert.AnError - }, - dataExchangeFunc: func(_ SerialPort, _ []byte) ([]byte, error) { - // Return valid NDEF data for first token - if callCount == 1 { - return []byte{ - 0x41, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - }, nil - } - return nil, assert.AnError - }, - } - - devicePath := testutils.CreateTempDevicePath(t) - device := config.ReadersConnect{ - Driver: "legacy_pn532_uart", - Path: devicePath, - } - - err := reader.Open(device, scanQueue, readers.OpenOpts{}) - require.NoError(t, err) - defer func() { - _ = reader.Close() - }() - - // First scan: token detected - scan1 := testutils.AssertScanReceived(t, scanQueue, 2*time.Second) - assert.NotNil(t, scan1.Token) - assert.Equal(t, "test-uid-123", scan1.Token.UID) - assert.NotEmpty(t, scan1.Token.ReaderID, "ReaderID must be set on tokens from hardware readers") - assert.False(t, scan1.ReaderError) - - // Wait for error count to exceed maxErrors (5) and ReaderError scan to be sent - // This tests the fix for issue #326 - ReaderError should be true - scan2 := testutils.AssertScanReceived(t, scanQueue, 5*time.Second) - assert.Nil(t, scan2.Token, "token should be nil on reader error") - assert.True(t, scan2.ReaderError, "ReaderError should be true to prevent on_remove execution") - - // Reader should have stopped polling - assert.True(t, testutils.WaitForCondition(func() bool { - return !reader.Connected() - }, 2*time.Second), "reader should have stopped after max errors") -} - -// TestOpen_ErrorCountExceedsMaxWithoutActiveToken verifies that ReaderError is sent -// even when there's no active token, to cancel any pending exit timers. -func TestOpen_ErrorCountExceedsMaxWithoutActiveToken(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test in short mode") - } - t.Parallel() - - cfg := &config.Instance{} - reader := NewReader(cfg) - scanQueue := testutils.CreateTestScanChannel(t) - - mockPort := &mockSerialPort{} - - // Connect function returns mock port - reader.connectFn = func(_ string) (SerialPort, error) { - return mockPort, nil - } - - // All calls fail to trigger error count without any successful token detection - reader.commander = &mockPN532Commander{ - listPassiveTargetFunc: func(_ SerialPort) (*Target, error) { - return nil, assert.AnError - }, - } - - devicePath := testutils.CreateTempDevicePath(t) - device := config.ReadersConnect{ - Driver: "legacy_pn532_uart", - Path: devicePath, - } - - err := reader.Open(device, scanQueue, readers.OpenOpts{}) - require.NoError(t, err) - defer func() { - _ = reader.Close() - }() - - // ReaderError should still be sent to cancel any pending exit timers - scan := testutils.AssertScanReceived(t, scanQueue, 5*time.Second) - assert.Nil(t, scan.Token) - assert.True(t, scan.ReaderError, "ReaderError should be sent even without active token") - - // Reader should have stopped polling - assert.True(t, testutils.WaitForCondition(func() bool { - return !reader.Connected() - }, 2*time.Second), "reader should have stopped after max errors") -} - -// TestOpen_TokenDetectionAndRemoval tests normal token detection and removal flow. -func TestOpen_TokenDetectionAndRemoval(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test in short mode") - } - t.Parallel() - - cfg := &config.Instance{} - reader := NewReader(cfg) - scanQueue := testutils.CreateTestScanChannel(t) - - mockPort := &mockSerialPort{} - callCount := 0 - maxZeroScans := 3 - - // Connect function returns mock port - reader.connectFn = func(_ string) (SerialPort, error) { - return mockPort, nil - } - - // Simulate: token detected → token present for a few scans → token removed - reader.commander = &mockPN532Commander{ - listPassiveTargetFunc: func(_ SerialPort) (*Target, error) { - callCount++ - // First 3 calls: token present - if callCount <= 3 { - return &Target{ - Type: tokens.TypeNTAG, - UID: "ntag-uid-456", - }, nil - } - // Next calls: token removed (nil) - return nil, nil //nolint:nilnil // nil target means no tag detected, which is valid behavior - }, - dataExchangeFunc: func(_ SerialPort, _ []byte) ([]byte, error) { - // Return empty NDEF data (will trigger empty block detection) - return []byte{ - 0x41, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - }, nil - }, - } - - devicePath := testutils.CreateTempDevicePath(t) - device := config.ReadersConnect{ - Driver: "legacy_pn532_uart", - Path: devicePath, - } - - err := reader.Open(device, scanQueue, readers.OpenOpts{}) - require.NoError(t, err) - defer func() { - _ = reader.Close() - }() - - // First scan: token detected - scan1 := testutils.AssertScanReceived(t, scanQueue, 2*time.Second) - assert.NotNil(t, scan1.Token) - assert.Equal(t, "ntag-uid-456", scan1.Token.UID) - assert.Equal(t, tokens.TypeNTAG, scan1.Token.Type) - assert.NotEmpty(t, scan1.Token.ReaderID, "ReaderID must be set on tokens from hardware readers") - assert.False(t, scan1.ReaderError) - - // Wait for token removal (maxZeroScans = 3) - // After 3 consecutive nil responses, token removal scan should be sent - scan2 := testutils.AssertScanReceived(t, scanQueue, time.Duration(maxZeroScans+2)*300*time.Millisecond) - assert.Nil(t, scan2.Token, "token should be nil on removal") - assert.False(t, scan2.ReaderError, "ReaderError should be false for normal removal") -} - -// TestOpen_MifareTokenRejected tests that Mifare tokens are rejected with a log message. -func TestOpen_MifareTokenRejected(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test in short mode") - } - t.Parallel() - - cfg := &config.Instance{} - reader := NewReader(cfg) - scanQueue := testutils.CreateTestScanChannel(t) - - mockPort := &mockSerialPort{} - callCount := 0 - - // Connect function returns mock port - reader.connectFn = func(_ string) (SerialPort, error) { - return mockPort, nil - } - - // Detect a Mifare token, then nil - reader.commander = &mockPN532Commander{ - listPassiveTargetFunc: func(_ SerialPort) (*Target, error) { - callCount++ - if callCount == 1 { - // First call: Mifare token (should be rejected) - return &Target{ - Type: tokens.TypeMifare, - UID: "mifare-uid-789", - }, nil - } - // Subsequent calls: no token - return nil, nil //nolint:nilnil // nil target means no tag detected, which is valid behavior - }, - } - - devicePath := testutils.CreateTempDevicePath(t) - device := config.ReadersConnect{ - Driver: "legacy_pn532_uart", - Path: devicePath, - } - - err := reader.Open(device, scanQueue, readers.OpenOpts{}) - require.NoError(t, err) - defer func() { - _ = reader.Close() - }() - - // No scan should be sent for Mifare token (it's rejected) - testutils.AssertNoScan(t, scanQueue, 2*time.Second) -} - -// TestOpen_DuplicateTokenIgnored tests that duplicate tokens are ignored. -func TestOpen_DuplicateTokenIgnored(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test in short mode") - } - t.Parallel() - - cfg := &config.Instance{} - reader := NewReader(cfg) - scanQueue := testutils.CreateTestScanChannel(t) - - mockPort := &mockSerialPort{} - - // Connect function returns mock port - reader.connectFn = func(_ string) (SerialPort, error) { - return mockPort, nil - } - - // Always return the same token - reader.commander = &mockPN532Commander{ - listPassiveTargetFunc: func(_ SerialPort) (*Target, error) { - return &Target{ - Type: tokens.TypeNTAG, - UID: "same-uid-999", - }, nil - }, - dataExchangeFunc: func(_ SerialPort, _ []byte) ([]byte, error) { - return []byte{ - 0x41, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - }, nil - }, - } - - devicePath := testutils.CreateTempDevicePath(t) - device := config.ReadersConnect{ - Driver: "legacy_pn532_uart", - Path: devicePath, - } - - err := reader.Open(device, scanQueue, readers.OpenOpts{}) - require.NoError(t, err) - defer func() { - _ = reader.Close() - }() - - // First scan: token detected - scan1 := testutils.AssertScanReceived(t, scanQueue, 2*time.Second) - assert.NotNil(t, scan1.Token) - assert.Equal(t, "same-uid-999", scan1.Token.UID) - assert.NotEmpty(t, scan1.Token.ReaderID, "ReaderID must be set on tokens from hardware readers") - - // No additional scans should be sent for the same token - testutils.AssertNoScan(t, scanQueue, 2*time.Second) -} - -// TestClose tests that Close() properly stops polling and closes the port. -func TestClose(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test in short mode") - } - t.Parallel() - - cfg := &config.Instance{} - reader := NewReader(cfg) - scanQueue := testutils.CreateTestScanChannel(t) - - mockPort := &mockSerialPort{} - - // Connect function returns mock port - reader.connectFn = func(_ string) (SerialPort, error) { - return mockPort, nil - } - - reader.commander = &mockPN532Commander{ - listPassiveTargetFunc: func(_ SerialPort) (*Target, error) { - return nil, nil //nolint:nilnil // nil target means no tag detected, which is valid behavior - }, - } - - devicePath := testutils.CreateTempDevicePath(t) - device := config.ReadersConnect{ - Driver: "legacy_pn532_uart", - Path: devicePath, - } - - err := reader.Open(device, scanQueue, readers.OpenOpts{}) - require.NoError(t, err) - - // Verify reader is connected - assert.True(t, reader.Connected()) - assert.True(t, reader.polling) - - // Close the reader - err = reader.Close() - require.NoError(t, err) - - // Verify reader is disconnected - assert.False(t, reader.Connected()) - assert.True(t, mockPort.IsClosed(), "port should be closed") -} diff --git a/pkg/readers/pn532uart/pn532uart_test.go b/pkg/readers/pn532uart/pn532uart_test.go deleted file mode 100644 index 66047bb93..000000000 --- a/pkg/readers/pn532uart/pn532uart_test.go +++ /dev/null @@ -1,140 +0,0 @@ -// Zaparoo Core -// Copyright (c) 2026 The Zaparoo Project Contributors. -// SPDX-License-Identifier: GPL-3.0-or-later -// -// This file is part of Zaparoo Core. -// -// Zaparoo Core is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// Zaparoo Core is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with Zaparoo Core. If not, see . - -package pn532uart - -import ( - "testing" - - "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/config" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestNewReader(t *testing.T) { - t.Parallel() - - cfg := &config.Instance{} - reader := NewReader(cfg) - - assert.NotNil(t, reader) - assert.Equal(t, cfg, reader.cfg) -} - -func TestMetadata(t *testing.T) { - t.Parallel() - - reader := &PN532UARTReader{} - metadata := reader.Metadata() - - assert.Equal(t, "legacypn532uart", metadata.ID) - assert.Equal(t, "Legacy PN532 UART reader", metadata.Description) - assert.True(t, metadata.DefaultEnabled) - assert.False(t, metadata.DefaultAutoDetect) -} - -func TestIDs(t *testing.T) { - t.Parallel() - - reader := &PN532UARTReader{} - ids := reader.IDs() - - require.Len(t, ids, 2) - assert.Equal(t, "legacypn532uart", ids[0]) - assert.Equal(t, "legacy_pn532_uart", ids[1]) -} - -func TestDetect(t *testing.T) { - t.Parallel() - - reader := &PN532UARTReader{} - result := reader.Detect([]string{"any", "input"}) - - // Detect() scans for available serial ports and attempts hardware detection. - // On systems with serial hardware, it may return a device connection string. - // On systems without serial ports, it returns empty string. - // Both are valid outcomes depending on hardware availability. - if result != "" { - // If a device was detected, verify it has the correct format - assert.Contains(t, result, "legacypn532uart:") - } -} - -func TestWrite_NotSupported(t *testing.T) { - t.Parallel() - - reader := &PN532UARTReader{} - token, err := reader.Write("test-data") - - assert.Nil(t, token) - require.Error(t, err) - assert.Contains(t, err.Error(), "writing not supported") -} - -func TestCancelWrite(t *testing.T) { - t.Parallel() - - reader := &PN532UARTReader{} - - // Should not panic - reader.CancelWrite() -} - -func TestCapabilities(t *testing.T) { - t.Parallel() - - reader := &PN532UARTReader{} - capabilities := reader.Capabilities() - - require.Len(t, capabilities, 1) - assert.Contains(t, capabilities, readers.CapabilityRemovable) -} - -func TestOnMediaChange(t *testing.T) { - t.Parallel() - - reader := &PN532UARTReader{} - err := reader.OnMediaChange(&models.ActiveMedia{}) - - assert.NoError(t, err, "OnMediaChange should return nil") -} - -func TestConnected_NoPort(t *testing.T) { - t.Parallel() - - reader := &PN532UARTReader{ - polling: true, - port: nil, - } - - assert.False(t, reader.Connected(), "should not be connected without port") -} - -func TestConnected_NotPolling(t *testing.T) { - t.Parallel() - - reader := &PN532UARTReader{ - polling: false, - port: nil, // Would need mock port - } - - assert.False(t, reader.Connected(), "should not be connected when not polling") -}