fix(mcp): reject over-max sample rates instead of clamping

src/Daqifi.Mcp/DaqifiAgent.cs

@@ -3,6 +3,7 @@
 using Daqifi.Core.Device;
 using Daqifi.Core.Device.Discovery;
 using Daqifi.Core.Device.SdCard;
+using Microsoft.Extensions.Logging;
 
 namespace Daqifi.Mcp;
 
@@ -24,11 +25,16 @@ namespace Daqifi.Mcp;
 public sealed class DaqifiAgent
 {
     private readonly ServerOptions _options;
+    private readonly ILogger<DaqifiAgent> _logger;
     private readonly ConcurrentDictionary<string, IDeviceInfo> _discovered = new(StringComparer.Ordinal);
     private readonly DaqifiDeviceRegistry _registry = new();
     private readonly SemaphoreSlim _gate = new(1, 1);
 
-    public DaqifiAgent(ServerOptions options) => _options = options;
+    public DaqifiAgent(ServerOptions options, ILogger<DaqifiAgent>? logger = null)
+    {
+        _options = options;
+        _logger = logger ?? Microsoft.Extensions.Logging.Abstractions.NullLogger<DaqifiAgent>.Instance;
+    }
 
     // ---------------------------------------------------------------- discovery
 
@@ -165,6 +171,11 @@ public async Task<ConfigureResult> ConfigureAnalogChannelsAsync(string deviceId,
                 streaming.EnableChannels(toEnable);
             }
 
+            // The device's authoritative rate cap (CapabilityStreaming.CurrentMaximumRateHz) is
+            // scoped to the channel set enabled when the document was read — refresh it now so
+            // set_sample_rate validates against the configuration that is actually live.
+            await RefreshCapabilityDocumentAsync(device, streaming).ConfigureAwait(false);
+
             return new ConfigureResult(deviceId, EnabledAnalog(device), streaming.StreamingFrequency);
         }
         finally
@@ -208,6 +219,10 @@ public async Task<ConfigureDigitalResult> ConfigureDigitalChannelsAsync(string d
                 streaming.EnableChannels(toEnable);
             }
 
+            // See ConfigureAnalogChannelsAsync: keeps CurrentMaximumRateHz current for
+            // set_sample_rate even though the rate model itself ignores digital channels.
+            await RefreshCapabilityDocumentAsync(device, streaming).ConfigureAwait(false);
+
             return new ConfigureDigitalResult(deviceId, EnabledDigital(device));
         }
         finally
@@ -343,21 +358,32 @@ public async Task<SampleRateResult> SetSampleRateAsync(string deviceId, int rate
             RequireControl();
             var (device, streaming) = RequireStreaming(deviceId);
 
-            // The device's advertised hardware ceiling always applies (Core validates
-            // StreamingFrequency against it too); --max-sample-rate-hz can only lower it.
-            // Guard against a non-positive cap so the applied rate is always a valid >= 1 value.
+            // MaxSamplingRate is the absolute sampling-ISR ceiling, not what the device will
+            // actually accept for the channels enabled right now — that is
+            // CapabilityStreaming.CurrentMaximumRateHz, refreshed after every channel-
+            // configuration call (see ConfigureAnalogChannelsAsync/ConfigureDigitalChannelsAsync).
+            // Guard against a non-positive board-table value so the fallback is always >= 1; a
+            // reported CurrentMaximumRateHz of 0 is a real answer ("no channels enabled") and is
+            // deliberately not floored the same way.
             var hardwareMax = Math.Max(1, device.Metadata.Capabilities.MaxSamplingRate);
-            var cap = _options.MaxSampleRateHz is { } max
-                ? Math.Clamp(max, 1, hardwareMax)
-                : hardwareMax;
 
-            var applied = Math.Min(rateHz, cap);
-            streaming.StreamingFrequency = applied;
+            // Bound to hardwareMax defensively: CurrentMaximumRateHz and MaxSamplingRate come from
+            // two independently-parsed fields, so a self-inconsistent document (or a channel-set
+            // read that raced a board-table update) could otherwise report a "current" cap above
+            // the absolute ceiling StreamingFrequency itself enforces — which would let this check
+            // pass and then fail one line down with the wrong exception type.
+            var currentMax = device.Metadata.CapabilityDocument?.Streaming?.CurrentMaximumRateHz;
+            var deviceCap = currentMax.HasValue ? Math.Min(currentMax.Value, hardwareMax) : hardwareMax;
+            var cap = _options.MaxSampleRateHz is { } max ? Math.Min(max, deviceCap) : deviceCap;
 
-            var note = applied != rateHz
-                ? $"Requested {rateHz} Hz clamped to {applied} Hz (maximum {cap} Hz)."
-                : null;
-            return new SampleRateResult(deviceId, rateHz, applied, applied != rateHz, note);
+            if (rateHz > cap)
+            {
+                throw new InvalidOperationException(
+                    $"Requested {rateHz} Hz exceeds the maximum {cap} Hz for the currently enabled channels.");
+            }
+
+            streaming.StreamingFrequency = rateHz;
+            return new SampleRateResult(deviceId, rateHz);
         }
         finally
         {
@@ -472,6 +498,34 @@ private DaqifiDevice Require(string deviceId)
         return (device, streaming);
     }
 
+    // Best-effort: a device that doesn't support the capability document (or a query that
+    // fails/times out) just leaves CurrentMaximumRateHz stale or absent, and SetSampleRateAsync
+    // falls back to the board-derived ceiling. Not fatal to the channel-configuration call that
+    // triggered the refresh. Skipped outright while streaming/logging: ReadCapabilityDocumentAsync
+    // runs a text-mode exchange that pauses the protobuf consumer, which Core documents as unsafe
+    // to call while streaming (SD logging sets IsStreaming too) — leaving the cap stale here is
+    // preferable to disrupting an active session.
+    private async Task RefreshCapabilityDocumentAsync(DaqifiDevice device, IStreamingDevice streaming)
+    {
+        if (streaming.IsStreaming)
+        {
+            return;
+        }
+
+        try
+        {
+            await device.ReadCapabilityDocumentAsync().ConfigureAwait(false);
+        }
+        catch (OperationCanceledException)
+        {
+            throw;
+        }
+        catch (Exception ex)
+        {
+            _logger.LogWarning(ex, "Capability-document refresh failed for '{DeviceId}'; set_sample_rate will keep using the last-known cap.", device.Metadata.SerialNumber);
+        }
+    }
+
     private static ISdCardOperations RequireSdCard(DaqifiDevice device)
     {
         if (device is not ISdCardOperations sd)

src/Daqifi.Mcp/Dtos.cs

@@ -140,11 +140,11 @@ public static PwmResult From(string deviceId, IStreamingDevice device, IChannel
 }
 
 /// <summary>
-/// Result of a sample-rate change. <see cref="Clamped"/> is true when <see cref="RequestedRateHz"/>
-/// exceeded the effective ceiling (1000 Hz hardware limit, or a lower <c>--max-sample-rate-hz</c>),
-/// in which case <see cref="Note"/> explains the adjustment.
+/// Result of a sample-rate change. A request exceeding the effective ceiling (the device's cap
+/// for its currently enabled channels, or a lower <c>--max-sample-rate-hz</c>) is rejected
+/// outright rather than clamped — see <see cref="DaqifiAgent.SetSampleRateAsync"/>.
 /// </summary>
-public sealed record SampleRateResult(string DeviceId, int RequestedRateHz, int AppliedRateHz, bool Clamped, string? Note);
+public sealed record SampleRateResult(string DeviceId, int RequestedRateHz);
 
 /// <summary>Result of starting SD-card logging.</summary>
 public sealed record StartLoggingResult(

src/Daqifi.Mcp/README.md

@@ -23,7 +23,7 @@ The server speaks MCP over **stdio**, so the client launches it as a subprocess.
 | `set_digital_output` | Drive a digital channel high or low (switches it to output if needed). |
 | `set_pwm_output` | Start PWM on a capable channel: duty 1-100%, shared frequency 6-50000 Hz. |
 | `disable_pwm` | Stop PWM on a channel (pin is left high-impedance). |
-| `set_sample_rate` | Set sample rate in Hz (Nyquist hardware supports up to 1000 Hz). |
+| `set_sample_rate` | Set sample rate in Hz (ceiling depends on the enabled channel count; over-cap requests are rejected). |
 | `start_sd_logging` | Start on-device SD logging (**requires a USB/serial connection**). |
 | `stop_sd_logging` | Stop SD logging. |
 
@@ -48,7 +48,7 @@ dotnet run --project src/Daqifi.Mcp
 
 ```
 --read-only               Expose discovery/introspection only; block configuration and logging.
---max-sample-rate-hz <n>  Clamp set_sample_rate to at most <n> Hz.
+--max-sample-rate-hz <n>  Reject set_sample_rate requests above <n> Hz.
 -h, --help                Show help.
 ```
 

src/Daqifi.Mcp/ServerOptions.cs

@@ -12,7 +12,8 @@ public sealed class ServerOptions
     public bool ReadOnly { get; init; }
 
     /// <summary>
-    /// Optional upper bound applied to <c>set_sample_rate</c>. Null means no clamp.
+    /// Optional upper bound enforced by <c>set_sample_rate</c>; requests above it are rejected.
+    /// Null means only the device's hardware ceiling applies.
     /// </summary>
     public int? MaxSampleRateHz { get; init; }
 
@@ -29,7 +30,7 @@ public static ServerOptions Parse(string[] args)
                     readOnly = true;
                     break;
                 case "--max-sample-rate-hz" when i + 1 < args.Length:
-                    // Ignore non-positive values; a clamp of <= 0 would otherwise force an invalid rate.
+                    // Ignore non-positive values; a cap of <= 0 would otherwise reject every rate.
                     if (int.TryParse(args[++i], out var rate) && rate >= 1)
                     {
                         maxRate = rate;
@@ -53,7 +54,7 @@ public static ServerOptions Parse(string[] args)
 
         Options:
           --read-only               Expose discovery/introspection only; block configuration and logging.
-          --max-sample-rate-hz <n>  Clamp set_sample_rate requests to at most <n> Hz.
+          --max-sample-rate-hz <n>  Reject set_sample_rate requests above <n> Hz.
           -h, --help                Show this help and exit.
         """;
 }

src/Daqifi.Mcp/Tools/DaqifiTools.cs

@@ -111,11 +111,11 @@ public static Task<PwmResult> DisablePwm(
         => GuardAsync(() => agent.DisablePwmAsync(deviceId, channel));
 
     [McpServerTool(Name = "set_sample_rate")]
-    [Description("Set the device sample (streaming) rate in Hz, applied to streaming and SD-card logging. Nyquist hardware supports 1–1000 Hz; requests above 1000 Hz (or above --max-sample-rate-hz) are clamped and reported in the result.")]
+    [Description("Set the device sample (streaming) rate in Hz, applied to streaming and SD-card logging. The achievable maximum depends on how many channels are currently enabled (configure channels first for an accurate ceiling) and is further capped by --max-sample-rate-hz if set; a request above the effective cap is rejected — the call throws rather than silently applying a lower rate.")]
     public static Task<SampleRateResult> SetSampleRate(
         DaqifiAgent agent,
         [Description("The device_id to configure.")] string deviceId,
-        [Description("Sample rate in Hz (1–1000).")] int rateHz)
+        [Description("Sample rate in Hz. The ceiling varies with the enabled channel count; get_device_status or a prior configure_analog_channels call error message reports the current limit.")] int rateHz)
         => GuardAsync(() => agent.SetSampleRateAsync(deviceId, rateHz));
 
     [McpServerTool(Name = "start_sd_logging")]