Merge pull request #14 from ZaparooProject/fix/windows-uart-fastread-lockup

fix: prevent PN532 firmware lockup on Windows UART with large FastRead operations

Author: wizzomafizzo

device.go

@@ -44,13 +44,18 @@ type DeviceConfig struct {
 	RetryConfig *RetryConfig
 	// Timeout is the default timeout for operations
 	Timeout time.Duration
+	// MaxFastReadPages limits the number of pages in a single FastRead operation
+	// Set to 0 to use platform-specific defaults (16 pages on Windows UART, unlimited elsewhere)
+	// This helps avoid PN532 firmware lockups with large InCommunicateThru payloads
+	MaxFastReadPages int
 }
 
 // DefaultDeviceConfig returns default device configuration
 func DefaultDeviceConfig() *DeviceConfig {
 	return &DeviceConfig{
-		RetryConfig: DefaultRetryConfig(),
-		Timeout:     1 * time.Second,
+		RetryConfig:      DefaultRetryConfig(),
+		Timeout:          1 * time.Second,
+		MaxFastReadPages: 0, // Use platform-specific defaults
 	}
 }
 

device_context.go

@@ -1067,3 +1067,17 @@ func (d *Device) PowerDownContext(ctx context.Context, wakeupEnable, irqEnable b
 
 	return nil
 }
+
+// ClearTransportState clears corrupted transport state to prevent firmware lockup
+// This is critical when switching between InCommunicateThru and InDataExchange
+// operations after frame reception failures
+func (d *Device) ClearTransportState() error {
+	// Check if the transport supports state clearing
+	if clearer, ok := d.transport.(interface{ ClearTransportState() error }); ok {
+		if err := clearer.ClearTransportState(); err != nil {
+			return fmt.Errorf("failed to clear transport state: %w", err)
+		}
+	}
+	// If transport doesn't support clearing, that's ok (some transports may not need it)
+	return nil
+}

ntag.go

@@ -25,6 +25,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"runtime"
 	"strings"
 	"time"
 )
@@ -245,6 +246,16 @@ func (t *NTAGTag) ReadNDEF() (*NDEFMessage, error) {
 
 	data, err := t.readNDEFDataWithFastRead(header, totalBytes)
 	if err != nil {
+		// CRITICAL: Clear transport state after failed InCommunicateThru operation
+		// This prevents firmware lockup when switching to InDataExchange protocol
+		fallbackReason := "FastRead failed"
+		if runtime.GOOS == "windows" && t.isUARTTransport() {
+			fallbackReason = "FastRead failed (Windows UART has size limits to prevent PN532 firmware lockup)"
+		}
+		debugf("NTAG %s, clearing transport state before fallback: %v", fallbackReason, err)
+		if clearErr := t.device.ClearTransportState(); clearErr != nil {
+			debugf("NTAG transport state clearing failed: %v", clearErr)
+		}
 		return t.readNDEFBlockByBlock()
 	}
 
@@ -339,6 +350,49 @@ func (t *NTAGTag) ensureTagTypeDetected() error {
 	return nil
 }
 
+// getMaxFastReadPages returns the maximum number of pages to read in a single FastRead operation
+// This is platform-specific to avoid PN532 firmware lockups with large InCommunicateThru payloads
+func (t *NTAGTag) getMaxFastReadPages() uint8 {
+	// Check if user has explicitly configured a limit
+	if t.device.config.MaxFastReadPages > 0 {
+		// Bounds check to prevent integer overflow and ensure reasonable limits
+		switch {
+		case t.device.config.MaxFastReadPages > 255:
+			debugf("NTAG MaxFastReadPages %d exceeds uint8 limit, capping to 255", t.device.config.MaxFastReadPages)
+			return 255
+		default:
+			// #nosec G115 - bounds checked above
+			return uint8(t.device.config.MaxFastReadPages)
+		}
+	}
+
+	// Apply platform-specific defaults
+	if runtime.GOOS == "windows" {
+		// Check if using UART transport which is most prone to the firmware lockup
+		if t.isUARTTransport() {
+			// 16 pages = 64 bytes - safe limit that avoids PN532 firmware lockups on Windows UART
+			// Based on research showing InCommunicateThru becomes unreliable beyond 64 bytes
+			debugf("NTAG applying Windows UART FastRead limit: 16 pages (64 bytes) to prevent firmware lockup")
+			debugf("NTAG tip: set config.MaxFastReadPages to override this Windows-specific safety limit")
+			return 16
+		}
+		debugf("NTAG Windows detected but non-UART transport, using default FastRead limits")
+	}
+
+	// Default: no limit (use original behavior)
+	return 60
+}
+
+// isUARTTransport checks if the current transport is UART-based
+func (t *NTAGTag) isUARTTransport() bool {
+	// Check if transport has UART capability
+	if checker, ok := t.device.transport.(TransportCapabilityChecker); ok {
+		return checker.HasCapability(CapabilityUART)
+	}
+	// Fallback: assume it might be UART to be safe
+	return true
+}
+
 func (t *NTAGTag) readNDEFDataWithFastRead(_ *ndefHeader, totalBytes int) ([]byte, error) {
 	if t.fastReadSupported != nil && !*t.fastReadSupported {
 		debugf("NTAG FastRead disabled - using block-by-block fallback")
@@ -348,7 +402,7 @@ func (t *NTAGTag) readNDEFDataWithFastRead(_ *ndefHeader, totalBytes int) ([]byt
 	debugf("NTAG attempting FastRead for NDEF data (%d bytes)", totalBytes)
 	readRange := t.calculateReadRange(totalBytes)
 
-	maxPagesPerRead := uint8(60)
+	maxPagesPerRead := t.getMaxFastReadPages()
 
 	if readRange.endPage-readRange.startPage+1 <= maxPagesPerRead {
 		debugf("NTAG using single FastRead for pages %d-%d", readRange.startPage, readRange.endPage)

ntag_test.go

@@ -539,4 +539,83 @@ func TestNTAG215BlockByBlockBufferOverflow(t *testing.T) {
 	}
 }
 
+// TestNTAGFastReadConfigLimits tests the FastRead configuration limits
+func TestNTAGFastReadConfigLimits(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name             string
+		maxFastReadPages int
+		expectedResult   uint8
+	}{
+		{
+			name:             "Custom config normal value",
+			maxFastReadPages: 32,
+			expectedResult:   32,
+		},
+		{
+			name:             "Custom config bounds check - exceeds uint8",
+			maxFastReadPages: 300,
+			expectedResult:   255, // Capped to uint8 max
+		},
+		{
+			name:             "Custom config at uint8 boundary",
+			maxFastReadPages: 255,
+			expectedResult:   255,
+		},
+		{
+			name:             "Custom config small value",
+			maxFastReadPages: 1,
+			expectedResult:   1,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			// Create device with test config
+			config := &DeviceConfig{
+				MaxFastReadPages: tt.maxFastReadPages,
+			}
+			device := &Device{
+				transport: &MockTransport{},
+				config:    config,
+			}
+
+			// Create NTAG tag
+			tag := NewNTAGTag(device, []byte{0x04, 0x12, 0x34, 0x56}, 0x00)
+
+			// Test the getMaxFastReadPages method
+			maxPages := tag.getMaxFastReadPages()
+
+			// Verify the result matches expected bounds checking
+			assert.Equal(t, tt.expectedResult, maxPages,
+				"MaxFastReadPages should respect config with proper bounds checking")
+		})
+	}
+}
+
+// TestNTAGFastReadDefaultLimits tests that default limits are reasonable
+func TestNTAGFastReadDefaultLimits(t *testing.T) {
+	t.Parallel()
+	// Create device with default config (0 = use platform defaults)
+	config := &DeviceConfig{
+		MaxFastReadPages: 0,
+	}
+	device := &Device{
+		transport: &MockTransport{},
+		config:    config,
+	}
+
+	// Create NTAG tag
+	tag := NewNTAGTag(device, []byte{0x04, 0x12, 0x34, 0x56}, 0x00)
+
+	// Test the getMaxFastReadPages method
+	maxPages := tag.getMaxFastReadPages()
+
+	// Verify the result is reasonable for any platform
+	// (should be either Windows UART limit of 16 or default of 60)
+	assert.True(t, maxPages == 16 || maxPages == 60,
+		"Default MaxFastReadPages should be either Windows UART limit (16) or default (60), got %d", maxPages)
+}
+
 // NDEF Message Tests

transport.go

@@ -74,6 +74,10 @@ const (
 	// CapabilityAutoPollNative indicates the transport supports native InAutoPoll
 	// with full command set and reliable operation (e.g., UART, I2C, SPI)
 	CapabilityAutoPollNative TransportCapability = "autopoll_native"
+
+	// CapabilityUART indicates the transport uses UART communication
+	// UART transport is prone to PN532 firmware lockups with large InCommunicateThru payloads
+	CapabilityUART TransportCapability = "uart"
 )
 
 // TransportCapabilityChecker defines an interface for querying transport capabilities

transport/uart/uart.go

@@ -228,6 +228,27 @@ func (*Transport) Type() pn532.TransportType {
 	return pn532.TransportUART
 }
 
+// ClearTransportState resets transport state after protocol failures
+// This is critical for preventing firmware lockup when switching between
+// InCommunicateThru and InDataExchange operations after frame corruption
+func (t *Transport) ClearTransportState() error {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+
+	if t.port != nil {
+		// Reset input buffer to clear any stale/corrupted data
+		_ = t.port.ResetInputBuffer()
+
+		// Add delay for Windows USB-UART drivers to process buffer reset
+		if isWindows() {
+			time.Sleep(15 * time.Millisecond)
+		} else {
+			time.Sleep(10 * time.Millisecond)
+		}
+	}
+	return nil
+}
+
 // isInterruptedSystemCall checks if an error is caused by an interrupted system call
 func isInterruptedSystemCall(err error) bool {
 	if err == nil {
@@ -782,6 +803,9 @@ func (*Transport) HasCapability(capability pn532.TransportCapability) bool {
 	case pn532.CapabilityRequiresInSelect:
 		// UART requires InSelect after InListPassiveTarget for proper target selection
 		return true
+	case pn532.CapabilityUART:
+		// This is the UART transport
+		return true
 	default:
 		return false
 	}