fix: address copilot review comments

- Guard GetRange/GetRangeStale against quantity=0 (false cache hit)
- Use shared coilOn/coilOff slices in decomposeResponse to reduce
  per-coil allocations
- Extract cleanupOnce so tests exercise the real keepStale guard
  instead of manually simulating cleanup

Author: tma

internal/cache/cache.go

@@ -132,6 +132,10 @@ func (c *Cache) Delete(key string) {
 // GetRange retrieves all values for a contiguous register range.
 // Returns the per-register/coil values and true only if ALL are cached and fresh.
 func (c *Cache) GetRange(slaveID byte, functionCode byte, startAddr uint16, quantity uint16) ([][]byte, bool) {
+	if quantity == 0 {
+		return nil, false
+	}
+
 	c.mu.RLock()
 	defer c.mu.RUnlock()
 
@@ -152,6 +156,10 @@ func (c *Cache) GetRange(slaveID byte, functionCode byte, startAddr uint16, quan
 // GetRangeStale retrieves all values for a contiguous register range, ignoring TTL.
 // Returns the per-register/coil values and true only if ALL are present (even if expired).
 func (c *Cache) GetRangeStale(slaveID byte, functionCode byte, startAddr uint16, quantity uint16) ([][]byte, bool) {
+	if quantity == 0 {
+		return nil, false
+	}
+
 	c.mu.RLock()
 	defer c.mu.RUnlock()
 
@@ -250,6 +258,21 @@ func (c *Cache) Coalesce(ctx context.Context, key string, fetch func(context.Con
 	return result, nil
 }
 
+// cleanupOnce runs a single cleanup pass, removing expired entries.
+// Skips deletion when keepStale is true.
+func (c *Cache) cleanupOnce() {
+	if c.keepStale {
+		return
+	}
+	c.mu.Lock()
+	for key, entry := range c.entries {
+		if entry.IsExpired() {
+			delete(c.entries, key)
+		}
+	}
+	c.mu.Unlock()
+}
+
 // cleanup periodically removes expired entries.
 func (c *Cache) cleanup() {
 	ticker := time.NewTicker(time.Minute)
@@ -260,16 +283,7 @@ func (c *Cache) cleanup() {
 		case <-c.done:
 			return
 		case <-ticker.C:
-			if c.keepStale {
-				continue
-			}
-			c.mu.Lock()
-			for key, entry := range c.entries {
-				if entry.IsExpired() {
-					delete(c.entries, key)
-				}
-			}
-			c.mu.Unlock()
+			c.cleanupOnce()
 		}
 	}
 }

internal/cache/cache_test.go

@@ -374,29 +374,24 @@ func TestCache_KeepStale(t *testing.T) {
 	c.Set("key1", []byte("value1"))
 	time.Sleep(100 * time.Millisecond)
 
-	// Simulate cleanup
-	c.mu.Lock()
-	for key, entry := range c.entries {
-		if entry.IsExpired() {
-			delete(c.entries, key)
-		}
-	}
-	c.mu.Unlock()
+	c.cleanupOnce()
 
 	if _, ok := c.GetStale("key1"); ok {
 		t.Error("expected stale data to be gone after cleanup with keepStale=false")
 	}
 	c.Close()
 
-	// With keepStale=true, expired entries survive cleanup
+	// With keepStale=true, cleanup skips deletion
 	c2 := New(50*time.Millisecond, true)
 	c2.Set("key1", []byte("value1"))
 	time.Sleep(100 * time.Millisecond)
 
-	// Entry should still be accessible via GetStale
+	c2.cleanupOnce()
+
+	// Entry should still be accessible via GetStale after cleanup
 	data, ok := c2.GetStale("key1")
 	if !ok {
-		t.Error("expected stale data to survive with keepStale=true")
+		t.Error("expected stale data to survive cleanup with keepStale=true")
 	}
 	if string(data) != "value1" {
 		t.Errorf("expected value1, got %s", string(data))

internal/proxy/proxy.go

@@ -202,6 +202,12 @@ func (p *Proxy) invalidateCache(req *modbus.Request) {
 	}
 }
 
+// Shared byte slices for coil values — safe to reuse since SetRange copies.
+var (
+	coilOn  = []byte{1}
+	coilOff = []byte{0}
+)
+
 // decomposeResponse extracts per-register/coil values from a Modbus read response.
 // Response format: [funcCode, byteCount, data...]
 // For registers (FC 0x03, 0x04): each register is 2 bytes.
@@ -236,9 +242,9 @@ func decomposeResponse(functionCode byte, quantity uint16, data []byte) [][]byte
 				return nil
 			}
 			if payload[byteIdx]&(1<<bitIdx) != 0 {
-				values[i] = []byte{1}
+				values[i] = coilOn
 			} else {
-				values[i] = []byte{0}
+				values[i] = coilOff
 			}
 		}
 		return values