From b9263a12c5decaac48c670fe5172214f598e5d7c Mon Sep 17 00:00:00 2001 From: Matt Draper Date: Fri, 20 Mar 2026 12:37:51 +0000 Subject: [PATCH 1/5] 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/5] 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/5] 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/5] 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/5] 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]