fix(device): parse protobuf field 22 so analog IsEnabled tracks the device (closes #409)

src/Daqifi.Core.Tests/Device/ChannelPopulationTests.cs

@@ -416,6 +416,127 @@ public void PopulateChannelsFromStatus_RepopulatingWithReportedResolution_Clears
         Assert.Equal(4095u, ((IAnalogChannel)newAnalog0).Resolution);
     }
 
+    [Fact]
+    public void PopulateChannelsFromStatus_WithAnalogInPortEnabled_SetsIsEnabledFromDevice()
+    {
+        // Arrange - device reports channels 0 and 2 enabled, 1 and 3 disabled via a bit-packed
+        // mask (byte 0b00000101 = 5): confirmed on the bench against a real Nq1 (fw 3.7.2),
+        // whose 16-channel status reported AnalogInPortEnabled as 2 bytes, [5,0], after enabling
+        // channels 0 and 2 — the same little-endian bit-per-channel layout Core sends outbound
+        // via EnableAdcChannels, not a byte-per-channel array (#409).
+        var device = new DaqifiDevice("TestDevice");
+        var message = new DaqifiOutMessage
+        {
+            AnalogInPortNum = 4,
+            AnalogInRes = 65535,
+            AnalogInPortEnabled = Google.Protobuf.ByteString.CopyFrom(0b0000_0101)
+        };
+
+        // Act
+        device.PopulateChannelsFromStatus(message);
+
+        // Assert
+        var analogChannels = device.Channels.Where(c => c.Type == ChannelType.Analog).ToList();
+        Assert.True(analogChannels[0].IsEnabled);
+        Assert.False(analogChannels[1].IsEnabled);
+        Assert.True(analogChannels[2].IsEnabled);
+        Assert.False(analogChannels[3].IsEnabled);
+    }
+
+    [Fact]
+    public void PopulateChannelsFromStatus_WithAnalogInPortEnabledSpanningMultipleBytes_SetsIsEnabledFromDevice()
+    {
+        // Arrange - a device with more than 8 analog channels packs the mask across multiple
+        // bytes; channel 9 is bit 1 of byte 1. Mirrors the bench-observed 2-byte mask for a
+        // 16-channel Nq1.
+        var device = new DaqifiDevice("TestDevice");
+        var message = new DaqifiOutMessage
+        {
+            AnalogInPortNum = 16,
+            AnalogInRes = 65535,
+            AnalogInPortEnabled = Google.Protobuf.ByteString.CopyFrom(0b0000_0000, 0b0000_0010)
+        };
+
+        // Act
+        device.PopulateChannelsFromStatus(message);
+
+        // Assert
+        var analogChannels = device.Channels.Where(c => c.Type == ChannelType.Analog).ToList();
+        Assert.False(analogChannels[0].IsEnabled);
+        Assert.True(analogChannels[9].IsEnabled);
+        Assert.False(analogChannels[10].IsEnabled);
+    }
+
+    [Fact]
+    public void PopulateChannelsFromStatus_WithoutAnalogInPortEnabled_DefaultsToDisabled()
+    {
+        // Arrange - older firmware never populates field 22; an empty byte string must not be
+        // read as "every channel disabled by the device" but simply "not reported".
+        var device = new DaqifiDevice("TestDevice");
+        var message = new DaqifiOutMessage
+        {
+            AnalogInPortNum = 2,
+            AnalogInRes = 65535
+        };
+
+        // Act
+        device.PopulateChannelsFromStatus(message);
+
+        // Assert
+        var analogChannels = device.Channels.Where(c => c.Type == ChannelType.Analog).ToList();
+        Assert.All(analogChannels, c => Assert.False(c.IsEnabled));
+    }
+
+    [Fact]
+    public void PopulateChannelsFromStatus_RepopulatingWithAnalogInPortEnabled_ResyncsExistingChannelFromDevice()
+    {
+        // A later status refresh must resync IsEnabled from the device's report rather than
+        // preserving whatever Core last set locally, so Core's view can't drift from the
+        // device's (#409).
+        var device = new DaqifiDevice("TestDevice");
+        var message = new DaqifiOutMessage { AnalogInPortNum = 2, AnalogInRes = 65535 };
+        device.PopulateChannelsFromStatus(message);
+
+        var analog0 = device.Channels.First(c => c.Type == ChannelType.Analog && c.ChannelNumber == 0);
+        analog0.IsEnabled = true; // Core enabled it locally.
+
+        // Act - the device reports channel 0 as actually disabled.
+        var refreshed = new DaqifiOutMessage
+        {
+            AnalogInPortNum = 2,
+            AnalogInRes = 65535,
+            AnalogInPortEnabled = Google.Protobuf.ByteString.CopyFrom(0b0000_0000)
+        };
+        device.PopulateChannelsFromStatus(refreshed);
+
+        // Assert - same instance, reused, but IsEnabled now reflects the device.
+        var newAnalog0 = device.Channels.First(c => c.Type == ChannelType.Analog && c.ChannelNumber == 0);
+        Assert.Same(analog0, newAnalog0);
+        Assert.False(newAnalog0.IsEnabled);
+    }
+
+    [Fact]
+    public void PopulateChannelsFromStatus_WithShorterAnalogInPortEnabled_TreatsMissingByteAsDisabled()
+    {
+        // Arrange - device reports fewer enabled-mask bytes than channels need (e.g. a truncated
+        // frame): channels 8+ have no byte to read and must default to disabled rather than throw.
+        var device = new DaqifiDevice("TestDevice");
+        var message = new DaqifiOutMessage
+        {
+            AnalogInPortNum = 9,
+            AnalogInRes = 65535,
+            AnalogInPortEnabled = Google.Protobuf.ByteString.CopyFrom(0b0000_0001)
+        };
+
+        // Act
+        device.PopulateChannelsFromStatus(message);
+
+        // Assert
+        var analogChannels = device.Channels.Where(c => c.Type == ChannelType.Analog).ToList();
+        Assert.True(analogChannels[0].IsEnabled);
+        Assert.All(analogChannels.Skip(1), c => Assert.False(c.IsEnabled));
+    }
+
     #endregion
 
     #region Digital Channel Population

src/Daqifi.Core.Tests/Device/DaqifiStreamingDeviceChannelManagementTests.cs

@@ -7,6 +7,8 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Net;
+using System.Threading;
+using System.Threading.Tasks;
 using Xunit;
 
 namespace Daqifi.Core.Tests.Device
@@ -239,6 +241,84 @@ public void DisableChannel_LastDigital_SendsGlobalDioDisable()
 
         #endregion
 
+        #region Concurrent status resync (#409)
+
+        [Fact]
+        public async Task EnableChannel_ConcurrentWithStatusResync_AlwaysSendsMaskIncludingJustEnabledChannel()
+        {
+            // Regression test for #409: PopulateChannelsFromStatus now resyncs analog IsEnabled
+            // from the device's reported mask on the consumer thread. Without a shared critical
+            // section, a status frame could interleave between EnableChannel's IsEnabled mutation
+            // and the ADC mask it computes and sends, silently dropping the just-enabled channel
+            // from the outbound mask. Hammer a concurrent status resync (reporting everything
+            // disabled) against repeated EnableChannel calls and assert the sent mask always
+            // reflects what was just requested.
+            var device = CreateConnectedDevice(analogChannels: 2, digitalChannels: 0);
+            var channel0 = AnalogChannelAt(device, 0);
+            var allDisabledStatus = new DaqifiOutMessage
+            {
+                AnalogInPortNum = 2,
+                AnalogInRes = 65535,
+                AnalogInPortEnabled = Google.Protobuf.ByteString.CopyFrom(0b0000_0000)
+            };
+
+            // Safety net so a regression that reintroduces a deadlock fails the test instead of
+            // hanging CI indefinitely.
+            using var stop = new CancellationTokenSource(TimeSpan.FromSeconds(30));
+            var resyncTask = Task.Run(() =>
+            {
+                var iteration = 0;
+                while (!stop.IsCancellationRequested)
+                {
+                    device.PopulateChannelsFromStatus(allDisabledStatus);
+                    // Yield periodically rather than every iteration — yielding every iteration
+                    // (e.g. via SpinWait.SpinOnce) spaces status frames out enough that this loop
+                    // stops reliably landing inside EnableChannel's now-much-shorter critical
+                    // section, silently defeating the regression coverage. Yielding every 64th
+                    // iteration keeps the interleaving pressure while still relinquishing the core
+                    // regularly enough to avoid pegging it.
+                    if (++iteration % 64 == 0)
+                    {
+                        Thread.Yield();
+                    }
+                }
+            });
+
+            try
+            {
+                for (var i = 0; i < 500; i++)
+                {
+                    device.DisableAllChannels();
+                    device.SentMessages.Clear();
+
+                    device.EnableChannel(channel0);
+
+                    var sent = Assert.Single(device.SentMessages);
+                    Assert.Equal(ScpiMessageProducer.EnableAdcChannels("1").Data, sent.Data);
+                }
+            }
+            finally
+            {
+                stop.Cancel();
+
+                // The cancellation token only stops the loop between iterations — it can't
+                // interrupt a PopulateChannelsFromStatus call already in progress. If a
+                // reintroduced deadlock ever wedges that call, awaiting resyncTask unbounded would
+                // hang CI instead of failing the test. Bound the wait so a deadlock regression
+                // fails deterministically instead.
+                try
+                {
+                    await resyncTask.WaitAsync(TimeSpan.FromSeconds(5));
+                }
+                catch (TimeoutException)
+                {
+                    Assert.Fail("The status-resync worker did not stop within 5s of cancellation — possible deadlock regression.");
+                }
+            }
+        }
+
+        #endregion
+
         #region DisableAllChannels
 
         [Fact]

src/Daqifi.Core/Device/DaqifiDevice.cs

@@ -270,6 +270,35 @@ public IReadOnlyList<IChannel> GetChannelsSnapshot()
         /// <inheritdoc cref="GetChannelsSnapshot"/>
         protected IReadOnlyList<IChannel> SnapshotChannels() => GetChannelsSnapshot();
 
+        /// <summary>
+        /// Runs <paramref name="action"/> under the same lock that guards structural access to
+        /// <see cref="_channels"/> and the status-driven <c>IsEnabled</c> resync in
+        /// <see cref="PopulateChannelsFromStatus"/>. A subclass's channel-management API (e.g.
+        /// enable/disable) should mutate <see cref="IChannel.IsEnabled"/> and compute any derived
+        /// outbound state (an ADC/DIO enable mask) inside this critical section, so a concurrent
+        /// status frame on the consumer thread cannot interleave between the mutation and the
+        /// read that derives the mask — which would send a mask computed from a value the status
+        /// frame is about to overwrite (#409). Callers must perform blocking I/O (e.g. <c>Send</c>)
+        /// outside this method; the lock is reentrant, so calling <see cref="SnapshotChannels"/>
+        /// from within <paramref name="action"/> is safe.
+        /// </summary>
+        protected T WithChannelsLock<T>(Func<T> action)
+        {
+            lock (_channelsLock)
+            {
+                return action();
+            }
+        }
+
+        /// <inheritdoc cref="WithChannelsLock{T}"/>
+        protected void WithChannelsLock(Action action)
+        {
+            lock (_channelsLock)
+            {
+                action();
+            }
+        }
+
         /// <summary>
         /// Gets the device's timestamp clock frequency in Hz.
         /// Populated from the <c>TimestampFreq</c> field of the status message.
@@ -1778,6 +1807,9 @@ private static void SafeLog(Action logAction)
         /// For analog channels, calibration parameters (CalM, CalB, InternalScaleM, PortRange)
         /// are extracted from the message. If there's a mismatch between the declared channel
         /// count and the available calibration data, default values are used for missing parameters.
+        /// Analog channel <c>IsEnabled</c> is likewise taken from the device-reported enabled mask
+        /// (field 22, <c>analog_in_port_enabled</c>) when the message carries one, so it reflects
+        /// the device's own view rather than only what Core previously commanded.
         ///
         /// For digital channels, only the channel count is used to create instances.
         /// </remarks>
@@ -1805,8 +1837,11 @@ public virtual void PopulateChannelsFromStatus(DaqifiOutMessage message)
             {
                 // Index existing channels by identity (type, number). Channels whose identity
                 // is unchanged are updated in place rather than replaced below, so consumer-held
-                // IChannel references — and the configuration on them (enable/direction/output/
-                // PWM state) — survive a routine status re-population untouched.
+                // IChannel references — and the configuration on them (direction/output/PWM
+                // state) — survive a routine status re-population untouched. IsEnabled is the
+                // exception: analog channels resync it from the device-reported enabled mask
+                // (field 22) whenever the device sends one, so Core's view cannot silently drift
+                // from the device's (#409).
                 var existingByKey = new Dictionary<(ChannelType, int), IChannel>();
                 foreach (var existing in _channels)
                 {
@@ -1857,6 +1892,12 @@ private int PopulateAnalogChannels(DaqifiOutMessage message, Dictionary<(Channel
             var analogInCalibrationMValues = message.AnalogInCalM;
             var analogInInternalScaleMValues = message.AnalogInIntScaleM;
             var analogInResolution = message.AnalogInRes;
+            var analogInPortEnabled = message.AnalogInPortEnabled;
+
+            // Firmware before v3.5.0 never populates this field, so an empty byte string is
+            // ambiguous between "no channels enabled" and "not reported". Only trust it as the
+            // source of truth for IsEnabled when the device actually sent something.
+            var enabledIsReported = analogInPortEnabled.Length > 0;
 
             var count = (int)message.AnalogInPortNum;
 
@@ -1894,6 +1935,10 @@ private int PopulateAnalogChannels(DaqifiOutMessage message, Dictionary<(Channel
                 if (existingByKey.TryGetValue((ChannelType.Analog, i), out var existing) && existing is AnalogChannel existingAnalog)
                 {
                     existingAnalog.UpdateScalingFromStatus(resolution, calibrationB, calibrationM, internalScaleM, portRange, resolutionIsAssumed);
+                    if (enabledIsReported)
+                    {
+                        existingAnalog.IsEnabled = IsChannelBitSet(analogInPortEnabled, i);
+                    }
                     destination.Add(existingAnalog);
                     continue;
                 }
@@ -1902,7 +1947,7 @@ private int PopulateAnalogChannels(DaqifiOutMessage message, Dictionary<(Channel
                 {
                     Name = $"AI{i}",
                     Direction = ChannelDirection.Input,
-                    IsEnabled = false,
+                    IsEnabled = enabledIsReported && IsChannelBitSet(analogInPortEnabled, i),
                     CalibrationB = calibrationB,
                     CalibrationM = calibrationM,
                     InternalScaleM = internalScaleM,
@@ -1994,6 +2039,19 @@ private int PopulateDigitalChannels(DaqifiOutMessage message, Dictionary<(Channe
             return count;
         }
 
+        /// <summary>
+        /// Reads bit <paramref name="channelNumber"/> from a device-reported per-channel enable
+        /// bitmask (analog_in_port_enabled, field 22 — confirmed bit-packed on the bench: 2 bytes
+        /// for 16 channels, little-endian, bit <c>n</c> = channel <c>n</c> — the same layout Core
+        /// sends outbound via <see cref="Communication.Producers.ScpiMessageProducer.EnableAdcChannels"/>).
+        /// Returns false when the channel number falls outside the bytes actually sent.
+        /// </summary>
+        private static bool IsChannelBitSet(Google.Protobuf.ByteString mask, int channelNumber)
+        {
+            var byteIndex = channelNumber / 8;
+            return byteIndex < mask.Length && (mask[byteIndex] & (1 << (channelNumber % 8))) != 0;
+        }
+
         /// <summary>
         /// Gets a value from a list with a default fallback if the index is out of range.
         /// </summary>