diff --git a/src/Daqifi.Mcp/DaqifiAgent.cs b/src/Daqifi.Mcp/DaqifiAgent.cs index e96b2a89..79a1bad6 100644 --- a/src/Daqifi.Mcp/DaqifiAgent.cs +++ b/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 _logger; private readonly ConcurrentDictionary _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? logger = null) + { + _options = options; + _logger = logger ?? Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance; + } // ---------------------------------------------------------------- discovery @@ -165,6 +171,11 @@ public async Task 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 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 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) diff --git a/src/Daqifi.Mcp/Dtos.cs b/src/Daqifi.Mcp/Dtos.cs index 9c05c001..6c7711a6 100644 --- a/src/Daqifi.Mcp/Dtos.cs +++ b/src/Daqifi.Mcp/Dtos.cs @@ -140,11 +140,11 @@ public static PwmResult From(string deviceId, IStreamingDevice device, IChannel } /// -/// Result of a sample-rate change. is true when -/// exceeded the effective ceiling (1000 Hz hardware limit, or a lower --max-sample-rate-hz), -/// in which case 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 --max-sample-rate-hz) is rejected +/// outright rather than clamped — see . /// -public sealed record SampleRateResult(string DeviceId, int RequestedRateHz, int AppliedRateHz, bool Clamped, string? Note); +public sealed record SampleRateResult(string DeviceId, int RequestedRateHz); /// Result of starting SD-card logging. public sealed record StartLoggingResult( diff --git a/src/Daqifi.Mcp/README.md b/src/Daqifi.Mcp/README.md index c97f39cf..007d9f6d 100644 --- a/src/Daqifi.Mcp/README.md +++ b/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 Clamp set_sample_rate to at most Hz. +--max-sample-rate-hz Reject set_sample_rate requests above Hz. -h, --help Show help. ``` diff --git a/src/Daqifi.Mcp/ServerOptions.cs b/src/Daqifi.Mcp/ServerOptions.cs index 3609b2cb..2ff8aa01 100644 --- a/src/Daqifi.Mcp/ServerOptions.cs +++ b/src/Daqifi.Mcp/ServerOptions.cs @@ -12,7 +12,8 @@ public sealed class ServerOptions public bool ReadOnly { get; init; } /// - /// Optional upper bound applied to set_sample_rate. Null means no clamp. + /// Optional upper bound enforced by set_sample_rate; requests above it are rejected. + /// Null means only the device's hardware ceiling applies. /// 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 Clamp set_sample_rate requests to at most Hz. + --max-sample-rate-hz Reject set_sample_rate requests above Hz. -h, --help Show this help and exit. """; } diff --git a/src/Daqifi.Mcp/Tools/DaqifiTools.cs b/src/Daqifi.Mcp/Tools/DaqifiTools.cs index c0874683..7f3ff2ac 100644 --- a/src/Daqifi.Mcp/Tools/DaqifiTools.cs +++ b/src/Daqifi.Mcp/Tools/DaqifiTools.cs @@ -111,11 +111,11 @@ public static Task 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 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")]