From b9263a12c5decaac48c670fe5172214f598e5d7c Mon Sep 17 00:00:00 2001 From: Matt Draper Date: Fri, 20 Mar 2026 12:37:51 +0000 Subject: [PATCH 1/7] Generate change detection factory for schemas with leveled bitfields For each schema with bitfield points that have level annotations, emit a static factory method that returns a BitfieldChangeDetector configured with the appropriate property getters and GetLevel calls. The runtime change detection logic lives in the consuming project. --- .../Generation/ModspecModelGenerator.cs | 33 ++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/Modspec.Model/Generation/ModspecModelGenerator.cs b/Modspec.Model/Generation/ModspecModelGenerator.cs index 913106f..0908c65 100644 --- a/Modspec.Model/Generation/ModspecModelGenerator.cs +++ b/Modspec.Model/Generation/ModspecModelGenerator.cs @@ -77,7 +77,8 @@ namespace {schema.Name}; List bufferInitialisers = []; List fieldInitialisers = []; List constructorParams = []; - WriteGroups(schema.Groups, mainWriter, appendixWriter, bufferInitialisers, fieldInitialisers, constructorParams); + List bitfieldPointsWithLevels = []; + WriteGroups(schema.Groups, mainWriter, appendixWriter, bufferInitialisers, fieldInitialisers, constructorParams, bitfieldPointsWithLevels: bitfieldPointsWithLevels); foreach (RepeatingGroup repeatingGroup in schema.RepeatingGroups) { @@ -137,6 +138,11 @@ namespace {schema.Name}; mainWriter.WriteLine("}"); mainWriter.WriteLine(); + if (bitfieldPointsWithLevels.Count > 0) + { + WriteChangeDetectionFactory(schema.Name, bitfieldPointsWithLevels, appendixWriter); + } + result = mainWriter.ToString() + appendixWriter.ToString(); return true; } @@ -168,7 +174,7 @@ private static void WriteFieldsAndConstructor(string name, StringWriter mainWrit mainWriter.WriteLine($"{indent}\t}}"); } - private static void WriteGroups(IReadOnlyCollection groups, StringWriter mainWriter, StringWriter appendixWriter, List bufferInitialisers, List fieldInitialisers, List constructorParams, string indent = "", string readOffsetField = "") + private static void WriteGroups(IReadOnlyCollection groups, StringWriter mainWriter, StringWriter appendixWriter, List bufferInitialisers, List fieldInitialisers, List constructorParams, string indent = "", string readOffsetField = "", List? bitfieldPointsWithLevels = null) { foreach (Group group in groups) { @@ -185,7 +191,7 @@ private static void WriteGroups(IReadOnlyCollection groups, StringWriter // supplied count of elements, rather than max size of array) throw new InvalidOperationException($"An array must be the last (or only) element in a group."); } - WritePoint(point, bufferName, group.Table, mainWriter, appendixWriter, fieldInitialisers, constructorParams, ref maxOffset, ref bufferSize, indent); + WritePoint(point, bufferName, group.Table, mainWriter, appendixWriter, fieldInitialisers, constructorParams, ref maxOffset, ref bufferSize, indent, bitfieldPointsWithLevels); } if (String.IsNullOrEmpty(bufferSize)) { @@ -209,7 +215,7 @@ private static void WriteGroups(IReadOnlyCollection groups, StringWriter } } - private static void WritePoint(Point point, string bufferName, Table table, StringWriter mainWriter, StringWriter appendixWriter, List fieldInitialisers, List constructorParams, ref int maxOffset, ref string bufferSize, string indent = "") + private static void WritePoint(Point point, string bufferName, Table table, StringWriter mainWriter, StringWriter appendixWriter, List fieldInitialisers, List constructorParams, ref int maxOffset, ref string bufferSize, string indent = "", List? bitfieldPointsWithLevels = null) { string type; string readMethod; @@ -337,6 +343,7 @@ private static void WritePoint(Point point, string bufferName, Table table, Stri appendixWriter.WriteLine(); if (isFlags && masksByLevel.Count > 0) { + bitfieldPointsWithLevels?.Add(point.Name); appendixWriter.WriteLine($"public static class {point.Name}Extensions"); appendixWriter.WriteLine("{"); appendixWriter.WriteLine($"\tpublic static Level GetLevel(this {point.Name} self)"); @@ -421,6 +428,24 @@ private static void WritePoint(Point point, string bufferName, Table table, Stri maxOffset += point.SizeInBytes * (point.Count?.MaxValue ?? 1); } + private static void WriteChangeDetectionFactory(string schemaName, List points, StringWriter writer) + { + string clientName = $"{schemaName}Client"; + writer.WriteLine($"public static class {schemaName}ChangeDetection"); + writer.WriteLine("{"); + writer.WriteLine($"\tpublic static BitfieldChangeDetector<{clientName}> CreateDetector()"); + writer.WriteLine("\t{"); + writer.WriteLine($"\t\treturn new BitfieldChangeDetector<{clientName}>()"); + for (int i = 0; i < points.Count; i++) + { + string terminator = i < points.Count - 1 ? "" : ";"; + writer.WriteLine($"\t\t\t.Track(c => c.{points[i]}, v => v.GetLevel()){terminator}"); + } + writer.WriteLine("\t}"); + writer.WriteLine("}"); + writer.WriteLine(); + } + private static string ToFieldName(string name) { Span result = stackalloc char[name.Length + 1]; From 77bd959e9a8491b7052bc4600e5d95299589cede Mon Sep 17 00:00:00 2001 From: Matt Draper Date: Fri, 20 Mar 2026 16:13:28 +0000 Subject: [PATCH 2/7] Add BitfieldChangeDetector stub for test project compilation The source generator now emits a change detection factory that references BitfieldChangeDetector, which lives in the consuming project. The test project needs a minimal stub so the generated code compiles. --- Modspec.Test/BitfieldChangeDetectorStub.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 Modspec.Test/BitfieldChangeDetectorStub.cs diff --git a/Modspec.Test/BitfieldChangeDetectorStub.cs b/Modspec.Test/BitfieldChangeDetectorStub.cs new file mode 100644 index 0000000..ff854a2 --- /dev/null +++ b/Modspec.Test/BitfieldChangeDetectorStub.cs @@ -0,0 +1,13 @@ +/* + * Stub so the generated change detection factory compiles in the test project, + * which does not reference the real BitfieldChangeDetector implementation. + */ +namespace Modspec.Model; + +public class BitfieldChangeDetector +{ + public BitfieldChangeDetector Track(System.Func getter, System.Func getLevel) where T : struct, System.Enum + { + return this; + } +} From 147d0c85eddf7e56e334e2da5829d6376fdf43d0 Mon Sep 17 00:00:00 2001 From: Matt Draper Date: Tue, 24 Mar 2026 16:59:47 +0000 Subject: [PATCH 3/7] creates a feature flag for the detectionfactory --- Modspec.Model/Generation/ModspecModelGenerator.cs | 2 +- Modspec.Model/Schema.cs | 13 +++++++++++++ Modspec.Test/BitfieldChangeDetectorStub.cs | 13 ------------- 3 files changed, 14 insertions(+), 14 deletions(-) delete mode 100644 Modspec.Test/BitfieldChangeDetectorStub.cs diff --git a/Modspec.Model/Generation/ModspecModelGenerator.cs b/Modspec.Model/Generation/ModspecModelGenerator.cs index 0908c65..f26cdd9 100644 --- a/Modspec.Model/Generation/ModspecModelGenerator.cs +++ b/Modspec.Model/Generation/ModspecModelGenerator.cs @@ -138,7 +138,7 @@ namespace {schema.Name}; mainWriter.WriteLine("}"); mainWriter.WriteLine(); - if (bitfieldPointsWithLevels.Count > 0) + if (schema.GenerateChangeDetectionFactory && bitfieldPointsWithLevels.Count > 0) { WriteChangeDetectionFactory(schema.Name, bitfieldPointsWithLevels, appendixWriter); } diff --git a/Modspec.Model/Schema.cs b/Modspec.Model/Schema.cs index 936818f..48502b2 100644 --- a/Modspec.Model/Schema.cs +++ b/Modspec.Model/Schema.cs @@ -48,6 +48,19 @@ public class Schema /// public List RepeatingGroups { get; set; } = []; + /// + /// If true, the generator will emit a static factory class for creating a + /// BitfieldChangeDetector that tracks all bitfield points with level annotations. + /// The consuming project must provide a BitfieldChangeDetector<TClient> class + /// in the Modspec.Model namespace with the following method: + /// + /// BitfieldChangeDetector<TClient> Track<T>(Func<TClient, T> getter, Func<T, Level> getLevel) + /// where T : struct, Enum + /// + /// The Track method must return the detector instance to support fluent chaining. + /// + public bool GenerateChangeDetectionFactory { get; set; } + public void Serialise(Stream stream) { JsonSerializer.Serialize(stream, this, Options); diff --git a/Modspec.Test/BitfieldChangeDetectorStub.cs b/Modspec.Test/BitfieldChangeDetectorStub.cs deleted file mode 100644 index ff854a2..0000000 --- a/Modspec.Test/BitfieldChangeDetectorStub.cs +++ /dev/null @@ -1,13 +0,0 @@ -/* - * Stub so the generated change detection factory compiles in the test project, - * which does not reference the real BitfieldChangeDetector implementation. - */ -namespace Modspec.Model; - -public class BitfieldChangeDetector -{ - public BitfieldChangeDetector Track(System.Func getter, System.Func getLevel) where T : struct, System.Enum - { - return this; - } -} From 61951aed34a3c16bf0c7e25e3e1681822f317cdb Mon Sep 17 00:00:00 2001 From: Matt Draper Date: Wed, 25 Mar 2026 16:08:04 +0000 Subject: [PATCH 4/7] adds tests and moves flag to csproj --- .../Generation/ModspecModelGenerator.cs | 24 ++++-- Modspec.Model/Schema.cs | 13 --- Modspec.Test/BitfieldChangeDetector.cs | 79 +++++++++++++++++++ Modspec.Test/Modspec.Test.csproj | 5 ++ Modspec.Test/Tests.cs | 64 +++++++++++++++ 5 files changed, 165 insertions(+), 20 deletions(-) create mode 100644 Modspec.Test/BitfieldChangeDetector.cs diff --git a/Modspec.Model/Generation/ModspecModelGenerator.cs b/Modspec.Model/Generation/ModspecModelGenerator.cs index f26cdd9..c42eb39 100644 --- a/Modspec.Model/Generation/ModspecModelGenerator.cs +++ b/Modspec.Model/Generation/ModspecModelGenerator.cs @@ -10,6 +10,7 @@ using System.Linq; using System.Text; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Text; using Modspec.Model.Extensions; @@ -23,12 +24,21 @@ public class ModelGenerator : IIncrementalGenerator { public void Initialize(IncrementalGeneratorInitializationContext context) { - var pipeline = context.AdditionalTextsProvider - .Where(static (file) => Path.GetFileName(file.Path).EndsWith("json")) - .Select(static (model, cancellationToken) => + var jsonFiles = context.AdditionalTextsProvider + .Where(static (file) => Path.GetFileName(file.Path).EndsWith("json")); + + var generateChangeDetection = context.AnalyzerConfigOptionsProvider + .Select(static (provider, _) => + { + provider.GlobalOptions.TryGetValue("build_property.ModspecGenerateChangeDetectionFactory", out string? value); + return String.Equals(value, "true", StringComparison.OrdinalIgnoreCase); + }); + + var pipeline = jsonFiles.Combine(generateChangeDetection) + .Select(static (pair, cancellationToken) => { - string path = model.Path; - ModelCompiler.TryGenerate(model.Path, out string? code); + string path = pair.Left.Path; + ModelCompiler.TryGenerate(path, pair.Right, out string? code); return (path, code); }) .Where(static (pair) => !String.IsNullOrEmpty(pair.code)); @@ -43,7 +53,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) private class ModelCompiler { - public static bool TryGenerate(string path, [NotNullWhen(true)] out string? result) + public static bool TryGenerate(string path, bool generateChangeDetectionFactory, [NotNullWhen(true)] out string? result) { result = default; Schema? schema; @@ -138,7 +148,7 @@ namespace {schema.Name}; mainWriter.WriteLine("}"); mainWriter.WriteLine(); - if (schema.GenerateChangeDetectionFactory && bitfieldPointsWithLevels.Count > 0) + if (generateChangeDetectionFactory && bitfieldPointsWithLevels.Count > 0) { WriteChangeDetectionFactory(schema.Name, bitfieldPointsWithLevels, appendixWriter); } diff --git a/Modspec.Model/Schema.cs b/Modspec.Model/Schema.cs index 48502b2..936818f 100644 --- a/Modspec.Model/Schema.cs +++ b/Modspec.Model/Schema.cs @@ -48,19 +48,6 @@ public class Schema /// public List RepeatingGroups { get; set; } = []; - /// - /// If true, the generator will emit a static factory class for creating a - /// BitfieldChangeDetector that tracks all bitfield points with level annotations. - /// The consuming project must provide a BitfieldChangeDetector<TClient> class - /// in the Modspec.Model namespace with the following method: - /// - /// BitfieldChangeDetector<TClient> Track<T>(Func<TClient, T> getter, Func<T, Level> getLevel) - /// where T : struct, Enum - /// - /// The Track method must return the detector instance to support fluent chaining. - /// - public bool GenerateChangeDetectionFactory { get; set; } - public void Serialise(Stream stream) { JsonSerializer.Serialize(stream, this, Options); diff --git a/Modspec.Test/BitfieldChangeDetector.cs b/Modspec.Test/BitfieldChangeDetector.cs new file mode 100644 index 0000000..3599617 --- /dev/null +++ b/Modspec.Test/BitfieldChangeDetector.cs @@ -0,0 +1,79 @@ +/* + * Simplified BitfieldChangeDetector for testing the generated factory. + * The real implementation lives in the consuming project + * and may use more sophisticated change tracking. + */ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; + +namespace Modspec.Model; + +public class BitfieldChangeDetector +{ + private readonly List _trackers = []; + + public BitfieldChangeDetector Track(Func getter, Func getLevel) where T : struct, Enum + { + _trackers.Add(new Tracker(getter, getLevel)); + return this; + } + + public async ValueTask CheckAsync(TClient client, Func onChanged) + { + bool anyChanged = false; + ulong combinedCode = 0; + Level highestLevel = Level.None; + List? descriptions = null; + + foreach (ITracker tracker in _trackers) + { + bool changed = tracker.TryCheck(client, out ulong code, out string desc, out Level level); + anyChanged |= changed; + combinedCode |= code; + if (level > highestLevel) highestLevel = level; + if (level != Level.None) + { + descriptions ??= []; + descriptions.Add(desc); + } + } + + if (anyChanged) + { + string combined = descriptions is not null + ? String.Join(", ", descriptions) + : String.Empty; + await onChanged(combinedCode, combined, highestLevel); + } + } + + private interface ITracker + { + bool TryCheck(TClient client, out ulong code, out string desc, out Level level); + } + + private class Tracker : ITracker where T : struct, Enum + { + private ulong _previous; + private readonly Func _getter; + private readonly Func _getLevel; + + public Tracker(Func getter, Func getLevel) + { + _getter = getter; + _getLevel = getLevel; + } + + public bool TryCheck(TClient client, out ulong code, out string desc, out Level level) + { + T current = _getter(client); + code = Convert.ToUInt64(current); + bool changed = code != _previous; + _previous = code; + level = _getLevel(current); + desc = current.ToString(); + return changed; + } + } +} diff --git a/Modspec.Test/Modspec.Test.csproj b/Modspec.Test/Modspec.Test.csproj index df0bdb7..13ba081 100644 --- a/Modspec.Test/Modspec.Test.csproj +++ b/Modspec.Test/Modspec.Test.csproj @@ -9,6 +9,7 @@ junit true true + true @@ -40,4 +41,8 @@ + + + + \ No newline at end of file diff --git a/Modspec.Test/Tests.cs b/Modspec.Test/Tests.cs index f97c020..a9f7093 100644 --- a/Modspec.Test/Tests.cs +++ b/Modspec.Test/Tests.cs @@ -87,6 +87,70 @@ public void TestErrorLevels() Assert.That(errors1.GetLevel(), Is.EqualTo(Level.Emergency)); } + [Test] + public void TestChangeDetectionFactoryCreatesDetector() + { + BitfieldChangeDetector detector = SomeBmsChangeDetection.CreateDetector(); + Assert.That(detector, Is.Not.Null); + } + + [Test] + public async Task TestChangeDetectionFactoryDetectsChange() + { + MockModbusClient mockClient = new MockModbusClient(); + SomeBmsClient bmsClient = new SomeBmsClient(mockClient, 2, 2, 480, 100); + BitfieldChangeDetector detector = SomeBmsChangeDetection.CreateDetector(); + + // initial read with no errors — check twice to confirm no spurious changes + await bmsClient.ReadWarningsErrorsEmergenciesAsync(); + bool called = false; + await detector.CheckAsync(bmsClient, (code, desc, level) => + { + called = true; + return ValueTask.CompletedTask; + }); + Assert.That(called, Is.False); + + // introduce an error and re-read + mockClient.DiscreteInputs.Span[1] = 0b10000000; // StringTerminalDischargeOverCurrentError + await bmsClient.ReadWarningsErrorsEmergenciesAsync(); + + Level reportedLevel = Level.None; + called = false; + await detector.CheckAsync(bmsClient, (code, desc, level) => + { + called = true; + reportedLevel = level; + return ValueTask.CompletedTask; + }); + Assert.That(called, Is.True); + Assert.That(reportedLevel, Is.EqualTo(Level.Error)); + } + + [Test] + public async Task TestChangeDetectionFactoryReportsHighestLevel() + { + MockModbusClient mockClient = new MockModbusClient(); + SomeBmsClient bmsClient = new SomeBmsClient(mockClient, 2, 2, 480, 100); + BitfieldChangeDetector detector = SomeBmsChangeDetection.CreateDetector(); + + // prime the detector + await bmsClient.ReadWarningsErrorsEmergenciesAsync(); + await detector.CheckAsync(bmsClient, (_, _, _) => ValueTask.CompletedTask); + + // set both a warning (bit 0) and an emergency (bit 2) on StringErrors1 + mockClient.DiscreteInputs.Span[1] = 0b00000101; + await bmsClient.ReadWarningsErrorsEmergenciesAsync(); + + Level reportedLevel = Level.None; + await detector.CheckAsync(bmsClient, (code, desc, level) => + { + reportedLevel = level; + return ValueTask.CompletedTask; + }); + Assert.That(reportedLevel, Is.EqualTo(Level.Emergency)); + } + [Test] public async Task TestRangeValidation() { From 0df1a3eeed9f30038f9c1ee935edf2a9b1a568e6 Mon Sep 17 00:00:00 2001 From: Matt Draper Date: Thu, 26 Mar 2026 17:13:38 +0000 Subject: [PATCH 5/7] in line change detection in the generated client class! --- .../Generation/ModspecModelGenerator.cs | 133 +++++++++++------- Modspec.Test/BitfieldChangeDetector.cs | 79 ----------- Modspec.Test/Modspec.Test.csproj | 5 - Modspec.Test/Tests.cs | 83 +++++------ 4 files changed, 125 insertions(+), 175 deletions(-) delete mode 100644 Modspec.Test/BitfieldChangeDetector.cs diff --git a/Modspec.Model/Generation/ModspecModelGenerator.cs b/Modspec.Model/Generation/ModspecModelGenerator.cs index c42eb39..28438c2 100644 --- a/Modspec.Model/Generation/ModspecModelGenerator.cs +++ b/Modspec.Model/Generation/ModspecModelGenerator.cs @@ -10,7 +10,6 @@ using System.Linq; using System.Text; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Text; using Modspec.Model.Extensions; @@ -24,21 +23,12 @@ public class ModelGenerator : IIncrementalGenerator { public void Initialize(IncrementalGeneratorInitializationContext context) { - var jsonFiles = context.AdditionalTextsProvider - .Where(static (file) => Path.GetFileName(file.Path).EndsWith("json")); - - var generateChangeDetection = context.AnalyzerConfigOptionsProvider - .Select(static (provider, _) => + var pipeline = context.AdditionalTextsProvider + .Where(static (file) => Path.GetFileName(file.Path).EndsWith("json")) + .Select(static (model, cancellationToken) => { - provider.GlobalOptions.TryGetValue("build_property.ModspecGenerateChangeDetectionFactory", out string? value); - return String.Equals(value, "true", StringComparison.OrdinalIgnoreCase); - }); - - var pipeline = jsonFiles.Combine(generateChangeDetection) - .Select(static (pair, cancellationToken) => - { - string path = pair.Left.Path; - ModelCompiler.TryGenerate(path, pair.Right, out string? code); + string path = model.Path; + ModelCompiler.TryGenerate(path, out string? code); return (path, code); }) .Where(static (pair) => !String.IsNullOrEmpty(pair.code)); @@ -53,7 +43,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) private class ModelCompiler { - public static bool TryGenerate(string path, bool generateChangeDetectionFactory, [NotNullWhen(true)] out string? result) + public static bool TryGenerate(string path, [NotNullWhen(true)] out string? result) { result = default; Schema? schema; @@ -69,6 +59,7 @@ public static bool TryGenerate(string path, bool generateChangeDetectionFactory, mainWriter.WriteLine($""" // This code has been generated by a tool. // Do not modify it. Your changes will be overwritten. +#nullable enable using System; using System.Buffers.Binary; using System.Collections.Generic; @@ -87,8 +78,8 @@ namespace {schema.Name}; List bufferInitialisers = []; List fieldInitialisers = []; List constructorParams = []; - List bitfieldPointsWithLevels = []; - WriteGroups(schema.Groups, mainWriter, appendixWriter, bufferInitialisers, fieldInitialisers, constructorParams, bitfieldPointsWithLevels: bitfieldPointsWithLevels); + bool hasChangeDetection = false; + WriteGroups(schema.Groups, mainWriter, appendixWriter, bufferInitialisers, fieldInitialisers, constructorParams, ref hasChangeDetection); foreach (RepeatingGroup repeatingGroup in schema.RepeatingGroups) { @@ -114,8 +105,18 @@ namespace {schema.Name}; mainWriter.WriteLine("\t\tprivate readonly IModbusClient _client;"); mainWriter.WriteLine("\t\tprivate readonly int _offset;"); // the offset to the base offset for this particular repeated element mainWriter.WriteLine(); - WriteGroups(repeatingGroup.Groups, mainWriter, appendixWriter, groupBufferInitialisers, groupFieldInitialisers, groupConstructorParams, "\t", "_offset + "); + bool groupHasChangeDetection = false; + WriteGroups(repeatingGroup.Groups, mainWriter, appendixWriter, groupBufferInitialisers, groupFieldInitialisers, groupConstructorParams, ref groupHasChangeDetection, "\t", "_offset + "); + if (groupHasChangeDetection) + { + hasChangeDetection = true; + mainWriter.WriteLine("\t\tprivate readonly Action? _onBitfieldChanged;"); + } groupFieldInitialisers.Add("_offset = offset;"); + if (groupHasChangeDetection) + { + groupFieldInitialisers.Add("_onBitfieldChanged = onBitfieldChanged;"); + } fieldInitialisers.Add($"{repeatingGroupFieldName} = new List<{repeatingGroup.Name}>();"); fieldInitialisers.Add($"for (int i = 0; i < {repeatingGroupCountParam.Name}; i++)"); fieldInitialisers.Add("{"); @@ -124,10 +125,11 @@ namespace {schema.Name}; { repeatingGroupParameterRefs = ", " + String.Join(", ", groupConstructorParams.Select(cp => cp.Name)); } - fieldInitialisers.Add($"\t{repeatingGroupFieldName}.Add(new {repeatingGroup.Name}(client, i * {repeatingGroup.Every}{repeatingGroupParameterRefs}));"); + string changeDetectionRef = groupHasChangeDetection ? ", onBitfieldChanged" : ""; + fieldInitialisers.Add($"\t{repeatingGroupFieldName}.Add(new {repeatingGroup.Name}(client, i * {repeatingGroup.Every}{repeatingGroupParameterRefs}{changeDetectionRef}));"); fieldInitialisers.Add("}"); groupConstructorParams.Insert(0, new("offset", UInt16.MaxValue)); - WriteFieldsAndConstructor(repeatingGroup.Name, mainWriter, groupBufferInitialisers, groupFieldInitialisers, groupConstructorParams, "\t"); + WriteFieldsAndConstructor(repeatingGroup.Name, mainWriter, groupBufferInitialisers, groupFieldInitialisers, groupConstructorParams, "\t", groupHasChangeDetection); mainWriter.WriteLine("\t}"); mainWriter.WriteLine(); // need to expose inner constructor parameters to parent client constructor @@ -144,21 +146,20 @@ namespace {schema.Name}; } } - WriteFieldsAndConstructor(schema.Name + "Client", mainWriter, bufferInitialisers, fieldInitialisers, constructorParams); - mainWriter.WriteLine("}"); - mainWriter.WriteLine(); - - if (generateChangeDetectionFactory && bitfieldPointsWithLevels.Count > 0) + if (hasChangeDetection) { - WriteChangeDetectionFactory(schema.Name, bitfieldPointsWithLevels, appendixWriter); + mainWriter.WriteLine("\tprivate readonly Action? _onBitfieldChanged;"); } + WriteFieldsAndConstructor(schema.Name + "Client", mainWriter, bufferInitialisers, fieldInitialisers, constructorParams, hasChangeDetection: hasChangeDetection); + mainWriter.WriteLine("}"); + mainWriter.WriteLine(); result = mainWriter.ToString() + appendixWriter.ToString(); return true; } } - private static void WriteFieldsAndConstructor(string name, StringWriter mainWriter, List bufferInitialisers, List fieldInitialisers, List constructorParams, string indent = "") + private static void WriteFieldsAndConstructor(string name, StringWriter mainWriter, List bufferInitialisers, List fieldInitialisers, List constructorParams, string indent = "", bool hasChangeDetection = false) { mainWriter.Write($"\t{indent}public {name}(IModbusClient client"); if (constructorParams.Count > 0) @@ -166,9 +167,17 @@ private static void WriteFieldsAndConstructor(string name, StringWriter mainWrit mainWriter.Write(", "); mainWriter.Write(String.Join(", ", constructorParams.Select(cp => $"int {cp.Name}"))); } + if (hasChangeDetection) + { + mainWriter.Write(", Action? onBitfieldChanged = null"); + } mainWriter.WriteLine(")"); mainWriter.WriteLine($"{indent}\t{{"); mainWriter.WriteLine($"{indent}\t\t_client = client;"); + if (hasChangeDetection) + { + mainWriter.WriteLine($"{indent}\t\t_onBitfieldChanged = onBitfieldChanged;"); + } foreach ((string constructorParamName, int maxCount) in constructorParams) { mainWriter.WriteLine($"{indent}\t\tif ({constructorParamName} > {maxCount}) throw new ArgumentException(\"{constructorParamName} is greater than the maximum permitted value ({maxCount}).\", \"{constructorParamName}\");"); @@ -184,14 +193,25 @@ private static void WriteFieldsAndConstructor(string name, StringWriter mainWrit mainWriter.WriteLine($"{indent}\t}}"); } - private static void WriteGroups(IReadOnlyCollection groups, StringWriter mainWriter, StringWriter appendixWriter, List bufferInitialisers, List fieldInitialisers, List constructorParams, string indent = "", string readOffsetField = "", List? bitfieldPointsWithLevels = null) + private static void WriteGroups(IReadOnlyCollection groups, StringWriter mainWriter, StringWriter appendixWriter, List bufferInitialisers, List fieldInitialisers, List constructorParams, ref bool hasChangeDetection, string indent = "", string readOffsetField = "") { foreach (Group group in groups) { string bufferName = $"_buffer{group.Name}"; + string previousName = $"_previous{group.Name}"; mainWriter.WriteLine($"{indent}\tprivate readonly Memory {bufferName};"); + // pre-scan to determine if this group has bitfield points with levels + bool groupHasLevels = group.Points.Any(p => + p.Type.IsBitfield() && p.Symbols is not null && + p.Symbols.Any(s => s.Level.HasValue && s.Level.Value != Level.None)); + if (groupHasLevels) + { + mainWriter.WriteLine($"{indent}\tprivate readonly Memory {previousName};"); + hasChangeDetection = true; + } int maxOffset = 0; string bufferSize = String.Empty; + List groupBitfieldPoints = []; for (int i = 0; i < group.Points.Count; i++) { Point point = group.Points[i]; @@ -201,31 +221,61 @@ private static void WriteGroups(IReadOnlyCollection groups, StringWriter // supplied count of elements, rather than max size of array) throw new InvalidOperationException($"An array must be the last (or only) element in a group."); } - WritePoint(point, bufferName, group.Table, mainWriter, appendixWriter, fieldInitialisers, constructorParams, ref maxOffset, ref bufferSize, indent, bitfieldPointsWithLevels); + WritePoint(point, bufferName, group.Table, mainWriter, appendixWriter, fieldInitialisers, constructorParams, ref maxOffset, ref bufferSize, indent, groupBitfieldPoints); } if (String.IsNullOrEmpty(bufferSize)) { bufferSize = $"{maxOffset}"; } bufferInitialisers.Add($"{bufferName} = new byte[{bufferSize}];"); + if (groupHasLevels) + { + bufferInitialisers.Add($"{previousName} = new byte[{bufferSize}];"); + } mainWriter.WriteLine(); // generate Read method for this group mainWriter.WriteLine($"\t{indent}public async ValueTask Read{group.Name}Async()"); mainWriter.WriteLine($"\t{indent}{{"); // note dependency between table name and Read...Async method on IModbusClient mainWriter.WriteLine($"\t\t{indent}await _client.Read{group.Table}Async({readOffsetField}{group.BaseRegister}, {bufferName});"); + if (groupBitfieldPoints.Count > 0) + { + mainWriter.WriteLine($"\t\t{indent}if (_onBitfieldChanged is not null) Check{group.Name}();"); + } mainWriter.WriteLine($"\t{indent}}}"); mainWriter.WriteLine(); mainWriter.WriteLine($"\t{indent}public void Read{group.Name}()"); mainWriter.WriteLine($"\t{indent}{{"); // note dependency between table name and Read... method on IModbusClient mainWriter.WriteLine($"\t\t{indent}_client.Read{group.Table}({readOffsetField}{group.BaseRegister}, {bufferName}.Span);"); + if (groupBitfieldPoints.Count > 0) + { + mainWriter.WriteLine($"\t\t{indent}if (_onBitfieldChanged is not null) Check{group.Name}();"); + } mainWriter.WriteLine($"\t{indent}}}"); mainWriter.WriteLine(); + // generate Check method for groups with leveled bitfields + if (groupBitfieldPoints.Count > 0) + { + mainWriter.WriteLine($"\t{indent}private void Check{group.Name}()"); + mainWriter.WriteLine($"\t{indent}{{"); + mainWriter.WriteLine($"\t\t{indent}Span current = {bufferName}.Span;"); + mainWriter.WriteLine($"\t\t{indent}Span previous = {previousName}.Span;"); + foreach (BitfieldPointInfo bp in groupBitfieldPoints) + { + mainWriter.WriteLine($"\t\t{indent}if (!current.Slice({bp.Offset}, {bp.SizeInBytes}).SequenceEqual(previous.Slice({bp.Offset}, {bp.SizeInBytes})))"); + mainWriter.WriteLine($"\t\t{indent}{{"); + mainWriter.WriteLine($"\t\t\t{indent}_onBitfieldChanged!(\"{bp.PointName}\", {bp.PointName}.GetLevel());"); + mainWriter.WriteLine($"\t\t{indent}}}"); + } + mainWriter.WriteLine($"\t\t{indent}current.CopyTo(previous);"); + mainWriter.WriteLine($"\t{indent}}}"); + mainWriter.WriteLine(); + } } } - private static void WritePoint(Point point, string bufferName, Table table, StringWriter mainWriter, StringWriter appendixWriter, List fieldInitialisers, List constructorParams, ref int maxOffset, ref string bufferSize, string indent = "", List? bitfieldPointsWithLevels = null) + private static void WritePoint(Point point, string bufferName, Table table, StringWriter mainWriter, StringWriter appendixWriter, List fieldInitialisers, List constructorParams, ref int maxOffset, ref string bufferSize, string indent = "", List? groupBitfieldPoints = null) { string type; string readMethod; @@ -353,7 +403,7 @@ private static void WritePoint(Point point, string bufferName, Table table, Stri appendixWriter.WriteLine(); if (isFlags && masksByLevel.Count > 0) { - bitfieldPointsWithLevels?.Add(point.Name); + groupBitfieldPoints?.Add(new BitfieldPointInfo(point.Name, maxOffset, point.SizeInBytes)); appendixWriter.WriteLine($"public static class {point.Name}Extensions"); appendixWriter.WriteLine("{"); appendixWriter.WriteLine($"\tpublic static Level GetLevel(this {point.Name} self)"); @@ -438,24 +488,6 @@ private static void WritePoint(Point point, string bufferName, Table table, Stri maxOffset += point.SizeInBytes * (point.Count?.MaxValue ?? 1); } - private static void WriteChangeDetectionFactory(string schemaName, List points, StringWriter writer) - { - string clientName = $"{schemaName}Client"; - writer.WriteLine($"public static class {schemaName}ChangeDetection"); - writer.WriteLine("{"); - writer.WriteLine($"\tpublic static BitfieldChangeDetector<{clientName}> CreateDetector()"); - writer.WriteLine("\t{"); - writer.WriteLine($"\t\treturn new BitfieldChangeDetector<{clientName}>()"); - for (int i = 0; i < points.Count; i++) - { - string terminator = i < points.Count - 1 ? "" : ";"; - writer.WriteLine($"\t\t\t.Track(c => c.{points[i]}, v => v.GetLevel()){terminator}"); - } - writer.WriteLine("\t}"); - writer.WriteLine("}"); - writer.WriteLine(); - } - private static string ToFieldName(string name) { Span result = stackalloc char[name.Length + 1]; @@ -503,4 +535,5 @@ private static string Pluralise(string self) } private record ConstructorParameter(string Name, int Count); + private record BitfieldPointInfo(string PointName, int Offset, int SizeInBytes); } \ No newline at end of file diff --git a/Modspec.Test/BitfieldChangeDetector.cs b/Modspec.Test/BitfieldChangeDetector.cs deleted file mode 100644 index 3599617..0000000 --- a/Modspec.Test/BitfieldChangeDetector.cs +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Simplified BitfieldChangeDetector for testing the generated factory. - * The real implementation lives in the consuming project - * and may use more sophisticated change tracking. - */ -using System; -using System.Collections.Generic; -using System.Threading.Tasks; - -namespace Modspec.Model; - -public class BitfieldChangeDetector -{ - private readonly List _trackers = []; - - public BitfieldChangeDetector Track(Func getter, Func getLevel) where T : struct, Enum - { - _trackers.Add(new Tracker(getter, getLevel)); - return this; - } - - public async ValueTask CheckAsync(TClient client, Func onChanged) - { - bool anyChanged = false; - ulong combinedCode = 0; - Level highestLevel = Level.None; - List? descriptions = null; - - foreach (ITracker tracker in _trackers) - { - bool changed = tracker.TryCheck(client, out ulong code, out string desc, out Level level); - anyChanged |= changed; - combinedCode |= code; - if (level > highestLevel) highestLevel = level; - if (level != Level.None) - { - descriptions ??= []; - descriptions.Add(desc); - } - } - - if (anyChanged) - { - string combined = descriptions is not null - ? String.Join(", ", descriptions) - : String.Empty; - await onChanged(combinedCode, combined, highestLevel); - } - } - - private interface ITracker - { - bool TryCheck(TClient client, out ulong code, out string desc, out Level level); - } - - private class Tracker : ITracker where T : struct, Enum - { - private ulong _previous; - private readonly Func _getter; - private readonly Func _getLevel; - - public Tracker(Func getter, Func getLevel) - { - _getter = getter; - _getLevel = getLevel; - } - - public bool TryCheck(TClient client, out ulong code, out string desc, out Level level) - { - T current = _getter(client); - code = Convert.ToUInt64(current); - bool changed = code != _previous; - _previous = code; - level = _getLevel(current); - desc = current.ToString(); - return changed; - } - } -} diff --git a/Modspec.Test/Modspec.Test.csproj b/Modspec.Test/Modspec.Test.csproj index 13ba081..df0bdb7 100644 --- a/Modspec.Test/Modspec.Test.csproj +++ b/Modspec.Test/Modspec.Test.csproj @@ -9,7 +9,6 @@ junit true true - true @@ -41,8 +40,4 @@ - - - - \ No newline at end of file diff --git a/Modspec.Test/Tests.cs b/Modspec.Test/Tests.cs index a9f7093..01e617c 100644 --- a/Modspec.Test/Tests.cs +++ b/Modspec.Test/Tests.cs @@ -1,5 +1,6 @@ using System; using System.Buffers.Binary; +using System.Collections.Generic; using System.IO; using System.Linq; using System.Reflection; @@ -88,67 +89,67 @@ public void TestErrorLevels() } [Test] - public void TestChangeDetectionFactoryCreatesDetector() + public async Task TestInlineChangeDetectionNoChange() { - BitfieldChangeDetector detector = SomeBmsChangeDetection.CreateDetector(); - Assert.That(detector, Is.Not.Null); + MockModbusClient mockClient = new MockModbusClient(); + List<(string name, Level level)> changes = []; + SomeBmsClient bmsClient = new SomeBmsClient(mockClient, 2, 2, 480, 100, + onBitfieldChanged: (name, level) => changes.Add((name, level))); + + // no errors — no callback + await bmsClient.ReadWarningsErrorsEmergenciesAsync(); + Assert.That(changes, Is.Empty); + + // read again with same state — still no callback + await bmsClient.ReadWarningsErrorsEmergenciesAsync(); + Assert.That(changes, Is.Empty); } [Test] - public async Task TestChangeDetectionFactoryDetectsChange() + public async Task TestInlineChangeDetectionDetectsChange() { MockModbusClient mockClient = new MockModbusClient(); - SomeBmsClient bmsClient = new SomeBmsClient(mockClient, 2, 2, 480, 100); - BitfieldChangeDetector detector = SomeBmsChangeDetection.CreateDetector(); + List<(string name, Level level)> changes = []; + SomeBmsClient bmsClient = new SomeBmsClient(mockClient, 2, 2, 480, 100, + onBitfieldChanged: (name, level) => changes.Add((name, level))); - // initial read with no errors — check twice to confirm no spurious changes - await bmsClient.ReadWarningsErrorsEmergenciesAsync(); - bool called = false; - await detector.CheckAsync(bmsClient, (code, desc, level) => - { - called = true; - return ValueTask.CompletedTask; - }); - Assert.That(called, Is.False); - - // introduce an error and re-read + // introduce an error and read mockClient.DiscreteInputs.Span[1] = 0b10000000; // StringTerminalDischargeOverCurrentError await bmsClient.ReadWarningsErrorsEmergenciesAsync(); + Assert.That(changes, Has.Count.EqualTo(1)); + Assert.That(changes[0].name, Is.EqualTo("StringErrors1")); + Assert.That(changes[0].level, Is.EqualTo(Level.Error)); - Level reportedLevel = Level.None; - called = false; - await detector.CheckAsync(bmsClient, (code, desc, level) => - { - called = true; - reportedLevel = level; - return ValueTask.CompletedTask; - }); - Assert.That(called, Is.True); - Assert.That(reportedLevel, Is.EqualTo(Level.Error)); + // same state again — no callback + changes.Clear(); + await bmsClient.ReadWarningsErrorsEmergenciesAsync(); + Assert.That(changes, Is.Empty); } [Test] - public async Task TestChangeDetectionFactoryReportsHighestLevel() + public async Task TestInlineChangeDetectionReportsHighestLevel() { MockModbusClient mockClient = new MockModbusClient(); - SomeBmsClient bmsClient = new SomeBmsClient(mockClient, 2, 2, 480, 100); - BitfieldChangeDetector detector = SomeBmsChangeDetection.CreateDetector(); - - // prime the detector - await bmsClient.ReadWarningsErrorsEmergenciesAsync(); - await detector.CheckAsync(bmsClient, (_, _, _) => ValueTask.CompletedTask); + List<(string name, Level level)> changes = []; + SomeBmsClient bmsClient = new SomeBmsClient(mockClient, 2, 2, 480, 100, + onBitfieldChanged: (name, level) => changes.Add((name, level))); // set both a warning (bit 0) and an emergency (bit 2) on StringErrors1 mockClient.DiscreteInputs.Span[1] = 0b00000101; await bmsClient.ReadWarningsErrorsEmergenciesAsync(); + Assert.That(changes, Has.Count.EqualTo(1)); + Assert.That(changes[0].level, Is.EqualTo(Level.Emergency)); + } - Level reportedLevel = Level.None; - await detector.CheckAsync(bmsClient, (code, desc, level) => - { - reportedLevel = level; - return ValueTask.CompletedTask; - }); - Assert.That(reportedLevel, Is.EqualTo(Level.Emergency)); + [Test] + public async Task TestNoCallbackNoOverhead() + { + // constructing without callback should work fine — no change detection runs + MockModbusClient mockClient = new MockModbusClient(); + SomeBmsClient bmsClient = new SomeBmsClient(mockClient, 2, 2, 480, 100); + mockClient.DiscreteInputs.Span[1] = 0b10000000; + await bmsClient.ReadWarningsErrorsEmergenciesAsync(); + Assert.That(bmsClient.StringErrors1, Is.EqualTo(StringErrors1.StringTerminalDischargeOverCurrentError)); } [Test] From e03d799d80fe2154de83e8b92568bec9c1e5b6ec Mon Sep 17 00:00:00 2001 From: Matt Draper Date: Fri, 27 Mar 2026 09:01:33 +0000 Subject: [PATCH 6/7] callback provides extra parameter so error bit is sent --- Modspec.Model/Generation/ModspecModelGenerator.cs | 8 ++++---- Modspec.Test/Tests.cs | 13 +++++++------ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/Modspec.Model/Generation/ModspecModelGenerator.cs b/Modspec.Model/Generation/ModspecModelGenerator.cs index 28438c2..29fed37 100644 --- a/Modspec.Model/Generation/ModspecModelGenerator.cs +++ b/Modspec.Model/Generation/ModspecModelGenerator.cs @@ -110,7 +110,7 @@ namespace {schema.Name}; if (groupHasChangeDetection) { hasChangeDetection = true; - mainWriter.WriteLine("\t\tprivate readonly Action? _onBitfieldChanged;"); + mainWriter.WriteLine("\t\tprivate readonly Action? _onBitfieldChanged;"); } groupFieldInitialisers.Add("_offset = offset;"); if (groupHasChangeDetection) @@ -148,7 +148,7 @@ namespace {schema.Name}; if (hasChangeDetection) { - mainWriter.WriteLine("\tprivate readonly Action? _onBitfieldChanged;"); + mainWriter.WriteLine("\tprivate readonly Action? _onBitfieldChanged;"); } WriteFieldsAndConstructor(schema.Name + "Client", mainWriter, bufferInitialisers, fieldInitialisers, constructorParams, hasChangeDetection: hasChangeDetection); mainWriter.WriteLine("}"); @@ -169,7 +169,7 @@ private static void WriteFieldsAndConstructor(string name, StringWriter mainWrit } if (hasChangeDetection) { - mainWriter.Write(", Action? onBitfieldChanged = null"); + mainWriter.Write(", Action? onBitfieldChanged = null"); } mainWriter.WriteLine(")"); mainWriter.WriteLine($"{indent}\t{{"); @@ -265,7 +265,7 @@ private static void WriteGroups(IReadOnlyCollection groups, StringWriter { mainWriter.WriteLine($"\t\t{indent}if (!current.Slice({bp.Offset}, {bp.SizeInBytes}).SequenceEqual(previous.Slice({bp.Offset}, {bp.SizeInBytes})))"); mainWriter.WriteLine($"\t\t{indent}{{"); - mainWriter.WriteLine($"\t\t\t{indent}_onBitfieldChanged!(\"{bp.PointName}\", {bp.PointName}.GetLevel());"); + mainWriter.WriteLine($"\t\t\t{indent}_onBitfieldChanged!(\"{bp.PointName}\", {bp.PointName}.ToString(), {bp.PointName}.GetLevel());"); mainWriter.WriteLine($"\t\t{indent}}}"); } mainWriter.WriteLine($"\t\t{indent}current.CopyTo(previous);"); diff --git a/Modspec.Test/Tests.cs b/Modspec.Test/Tests.cs index 01e617c..5c81666 100644 --- a/Modspec.Test/Tests.cs +++ b/Modspec.Test/Tests.cs @@ -92,9 +92,9 @@ public void TestErrorLevels() public async Task TestInlineChangeDetectionNoChange() { MockModbusClient mockClient = new MockModbusClient(); - List<(string name, Level level)> changes = []; + List<(string name, string description, Level level)> changes = []; SomeBmsClient bmsClient = new SomeBmsClient(mockClient, 2, 2, 480, 100, - onBitfieldChanged: (name, level) => changes.Add((name, level))); + onBitfieldChanged: (name, desc, level) => changes.Add((name, desc, level))); // no errors — no callback await bmsClient.ReadWarningsErrorsEmergenciesAsync(); @@ -109,15 +109,16 @@ public async Task TestInlineChangeDetectionNoChange() public async Task TestInlineChangeDetectionDetectsChange() { MockModbusClient mockClient = new MockModbusClient(); - List<(string name, Level level)> changes = []; + List<(string name, string description, Level level)> changes = []; SomeBmsClient bmsClient = new SomeBmsClient(mockClient, 2, 2, 480, 100, - onBitfieldChanged: (name, level) => changes.Add((name, level))); + onBitfieldChanged: (name, desc, level) => changes.Add((name, desc, level))); // introduce an error and read mockClient.DiscreteInputs.Span[1] = 0b10000000; // StringTerminalDischargeOverCurrentError await bmsClient.ReadWarningsErrorsEmergenciesAsync(); Assert.That(changes, Has.Count.EqualTo(1)); Assert.That(changes[0].name, Is.EqualTo("StringErrors1")); + Assert.That(changes[0].description, Does.Contain("StringTerminalDischargeOverCurrentError")); Assert.That(changes[0].level, Is.EqualTo(Level.Error)); // same state again — no callback @@ -130,9 +131,9 @@ public async Task TestInlineChangeDetectionDetectsChange() public async Task TestInlineChangeDetectionReportsHighestLevel() { MockModbusClient mockClient = new MockModbusClient(); - List<(string name, Level level)> changes = []; + List<(string name, string description, Level level)> changes = []; SomeBmsClient bmsClient = new SomeBmsClient(mockClient, 2, 2, 480, 100, - onBitfieldChanged: (name, level) => changes.Add((name, level))); + onBitfieldChanged: (name, desc, level) => changes.Add((name, desc, level))); // set both a warning (bit 0) and an emergency (bit 2) on StringErrors1 mockClient.DiscreteInputs.Span[1] = 0b00000101; From beb60fbfd238d5d58d38966ed89d6958938afd87 Mon Sep 17 00:00:00 2001 From: Matt Draper Date: Mon, 30 Mar 2026 08:54:44 +0100 Subject: [PATCH 7/7] Change the callback signature from Action to a named BitfieldChangedCallback delegate declared alongside IModbusClient. The callback now passes both the previous and current enum values as boxed objects instead of a pre-computed description string. Consumers can call ToString() for flag names, cast to the typed enum, or convert to ulong for the numeric value. Add braces around all generated if statement bodies. --- .../Generation/ModspecModelGenerator.cs | 23 ++++++++++++------- Modspec.Model/IModbusClient.cs | 9 ++++++++ Modspec.Test/Tests.cs | 16 +++++++------ 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/Modspec.Model/Generation/ModspecModelGenerator.cs b/Modspec.Model/Generation/ModspecModelGenerator.cs index 29fed37..df10b44 100644 --- a/Modspec.Model/Generation/ModspecModelGenerator.cs +++ b/Modspec.Model/Generation/ModspecModelGenerator.cs @@ -110,7 +110,7 @@ namespace {schema.Name}; if (groupHasChangeDetection) { hasChangeDetection = true; - mainWriter.WriteLine("\t\tprivate readonly Action? _onBitfieldChanged;"); + mainWriter.WriteLine("\t\tprivate readonly BitfieldChangedCallback? _onBitfieldChanged;"); } groupFieldInitialisers.Add("_offset = offset;"); if (groupHasChangeDetection) @@ -148,7 +148,7 @@ namespace {schema.Name}; if (hasChangeDetection) { - mainWriter.WriteLine("\tprivate readonly Action? _onBitfieldChanged;"); + mainWriter.WriteLine("\tprivate readonly BitfieldChangedCallback? _onBitfieldChanged;"); } WriteFieldsAndConstructor(schema.Name + "Client", mainWriter, bufferInitialisers, fieldInitialisers, constructorParams, hasChangeDetection: hasChangeDetection); mainWriter.WriteLine("}"); @@ -169,7 +169,7 @@ private static void WriteFieldsAndConstructor(string name, StringWriter mainWrit } if (hasChangeDetection) { - mainWriter.Write(", Action? onBitfieldChanged = null"); + mainWriter.Write(", BitfieldChangedCallback? onBitfieldChanged = null"); } mainWriter.WriteLine(")"); mainWriter.WriteLine($"{indent}\t{{"); @@ -240,7 +240,10 @@ private static void WriteGroups(IReadOnlyCollection groups, StringWriter mainWriter.WriteLine($"\t\t{indent}await _client.Read{group.Table}Async({readOffsetField}{group.BaseRegister}, {bufferName});"); if (groupBitfieldPoints.Count > 0) { - mainWriter.WriteLine($"\t\t{indent}if (_onBitfieldChanged is not null) Check{group.Name}();"); + mainWriter.WriteLine($"\t\t{indent}if (_onBitfieldChanged is not null)"); + mainWriter.WriteLine($"\t\t{indent}{{"); + mainWriter.WriteLine($"\t\t\t{indent}Check{group.Name}();"); + mainWriter.WriteLine($"\t\t{indent}}}"); } mainWriter.WriteLine($"\t{indent}}}"); mainWriter.WriteLine(); @@ -250,7 +253,10 @@ private static void WriteGroups(IReadOnlyCollection groups, StringWriter mainWriter.WriteLine($"\t\t{indent}_client.Read{group.Table}({readOffsetField}{group.BaseRegister}, {bufferName}.Span);"); if (groupBitfieldPoints.Count > 0) { - mainWriter.WriteLine($"\t\t{indent}if (_onBitfieldChanged is not null) Check{group.Name}();"); + mainWriter.WriteLine($"\t\t{indent}if (_onBitfieldChanged is not null)"); + mainWriter.WriteLine($"\t\t{indent}{{"); + mainWriter.WriteLine($"\t\t\t{indent}Check{group.Name}();"); + mainWriter.WriteLine($"\t\t{indent}}}"); } mainWriter.WriteLine($"\t{indent}}}"); mainWriter.WriteLine(); @@ -265,7 +271,8 @@ private static void WriteGroups(IReadOnlyCollection groups, StringWriter { mainWriter.WriteLine($"\t\t{indent}if (!current.Slice({bp.Offset}, {bp.SizeInBytes}).SequenceEqual(previous.Slice({bp.Offset}, {bp.SizeInBytes})))"); mainWriter.WriteLine($"\t\t{indent}{{"); - mainWriter.WriteLine($"\t\t\t{indent}_onBitfieldChanged!(\"{bp.PointName}\", {bp.PointName}.ToString(), {bp.PointName}.GetLevel());"); + mainWriter.WriteLine($"\t\t\t{indent}{bp.PointName} oldValue = ({bp.PointName}){bp.ReadMethod}(previous.Slice({bp.Offset}, {bp.SizeInBytes}));"); + mainWriter.WriteLine($"\t\t\t{indent}_onBitfieldChanged!(\"{bp.PointName}\", oldValue, {bp.PointName}, {bp.PointName}.GetLevel());"); mainWriter.WriteLine($"\t\t{indent}}}"); } mainWriter.WriteLine($"\t\t{indent}current.CopyTo(previous);"); @@ -403,7 +410,7 @@ private static void WritePoint(Point point, string bufferName, Table table, Stri appendixWriter.WriteLine(); if (isFlags && masksByLevel.Count > 0) { - groupBitfieldPoints?.Add(new BitfieldPointInfo(point.Name, maxOffset, point.SizeInBytes)); + groupBitfieldPoints?.Add(new BitfieldPointInfo(point.Name, maxOffset, point.SizeInBytes, readMethod)); appendixWriter.WriteLine($"public static class {point.Name}Extensions"); appendixWriter.WriteLine("{"); appendixWriter.WriteLine($"\tpublic static Level GetLevel(this {point.Name} self)"); @@ -535,5 +542,5 @@ private static string Pluralise(string self) } private record ConstructorParameter(string Name, int Count); - private record BitfieldPointInfo(string PointName, int Offset, int SizeInBytes); + private record BitfieldPointInfo(string PointName, int Offset, int SizeInBytes, string ReadMethod); } \ No newline at end of file diff --git a/Modspec.Model/IModbusClient.cs b/Modspec.Model/IModbusClient.cs index ec27a1e..bf487fa 100644 --- a/Modspec.Model/IModbusClient.cs +++ b/Modspec.Model/IModbusClient.cs @@ -8,6 +8,15 @@ namespace Modspec.Model; +/// +/// Callback invoked when a bitfield point with level annotations changes between reads. +/// +/// The name of the bitfield point that changed. +/// The previous value of the bitfield (boxed enum). +/// The current value of the bitfield (boxed enum). +/// The highest severity level among the currently set flags. +public delegate void BitfieldChangedCallback(string name, object oldValue, object newValue, Level level); + /// /// Interface for a Modbus client. /// diff --git a/Modspec.Test/Tests.cs b/Modspec.Test/Tests.cs index 5c81666..e849a64 100644 --- a/Modspec.Test/Tests.cs +++ b/Modspec.Test/Tests.cs @@ -92,9 +92,9 @@ public void TestErrorLevels() public async Task TestInlineChangeDetectionNoChange() { MockModbusClient mockClient = new MockModbusClient(); - List<(string name, string description, Level level)> changes = []; + List<(string name, object oldValue, object newValue, Level level)> changes = []; SomeBmsClient bmsClient = new SomeBmsClient(mockClient, 2, 2, 480, 100, - onBitfieldChanged: (name, desc, level) => changes.Add((name, desc, level))); + onBitfieldChanged: (name, oldVal, newVal, level) => changes.Add((name, oldVal, newVal, level))); // no errors — no callback await bmsClient.ReadWarningsErrorsEmergenciesAsync(); @@ -109,16 +109,18 @@ public async Task TestInlineChangeDetectionNoChange() public async Task TestInlineChangeDetectionDetectsChange() { MockModbusClient mockClient = new MockModbusClient(); - List<(string name, string description, Level level)> changes = []; + List<(string name, object oldValue, object newValue, Level level)> changes = []; SomeBmsClient bmsClient = new SomeBmsClient(mockClient, 2, 2, 480, 100, - onBitfieldChanged: (name, desc, level) => changes.Add((name, desc, level))); + onBitfieldChanged: (name, oldVal, newVal, level) => changes.Add((name, oldVal, newVal, level))); // introduce an error and read mockClient.DiscreteInputs.Span[1] = 0b10000000; // StringTerminalDischargeOverCurrentError await bmsClient.ReadWarningsErrorsEmergenciesAsync(); Assert.That(changes, Has.Count.EqualTo(1)); Assert.That(changes[0].name, Is.EqualTo("StringErrors1")); - Assert.That(changes[0].description, Does.Contain("StringTerminalDischargeOverCurrentError")); + Assert.That(changes[0].oldValue, Is.EqualTo((StringErrors1)0)); + Assert.That(changes[0].newValue, Is.EqualTo(StringErrors1.StringTerminalDischargeOverCurrentError)); + Assert.That(changes[0].newValue.ToString(), Does.Contain("StringTerminalDischargeOverCurrentError")); Assert.That(changes[0].level, Is.EqualTo(Level.Error)); // same state again — no callback @@ -131,9 +133,9 @@ public async Task TestInlineChangeDetectionDetectsChange() public async Task TestInlineChangeDetectionReportsHighestLevel() { MockModbusClient mockClient = new MockModbusClient(); - List<(string name, string description, Level level)> changes = []; + List<(string name, object oldValue, object newValue, Level level)> changes = []; SomeBmsClient bmsClient = new SomeBmsClient(mockClient, 2, 2, 480, 100, - onBitfieldChanged: (name, desc, level) => changes.Add((name, desc, level))); + onBitfieldChanged: (name, oldVal, newVal, level) => changes.Add((name, oldVal, newVal, level))); // set both a warning (bit 0) and an emergency (bit 2) on StringErrors1 mockClient.DiscreteInputs.Span[1] = 0b00000101;