Repository: intel/rohd (dart)

Issue:
"Default" values for `FiniteStateMachine`s
### Motivation

Often there is a "default" value that would be desirable for most states in a `FiniteStateMachine`, but the current implementation requires repeating the same default across all states in order to avoid an inferred latch.

### Desired solution

Upgrade the `FiniteStateMachine` API to support some way to add defaults.

This could easily be done by inserting conditional assignments (since they act as "blocking") before the rest of the combinational implementation in the `Combinational` that it generates.

### Alternatives considered

_No response_

### Additional details

_No response_

Pull request description:
<!-- Please make sure you check out the contribution guidelines before submitting a pull request! -->

## Description & Motivation

This PR has a handful of upgrades to `FiniteStateMachine`s so that it is easier to construct and generate readable FSM code.

- Add labels in generated verilog for FSMs (the final version of this should use #585)
  - this was removed from this PR, we can leave it to #585 to handle the labels
- Add a way to set up defaults across all states (namely via `setupActions`), addressing #509
- Perhaps: addressing #313, still thinking about how/if to do it here or as a separate component/abstraction
  - an implementation with updates was opened in a PR in ROHD-HCL, but not sure if it will get merged: https://github.com/intel/rohd-hcl/pull/196

This PR also includes some misc lint cleanups for newer versions of Dart

## Related Issue(s)

Fix #509

## Testing

Updated test for lights to leverage it, confirmed failure if the feature was broken.  Added new tests for other stuff.

Existing tests cover much, but need to add more for new features

## Backwards-compatibility

> Is this a breaking change that will not be backwards-compatible? If yes, how so?

No

## Documentation

> Does the change require any updates to documentation? If so, where? Are they included?

Updated examples.

Patch:
diff --git a/.github/workflows/general.yml b/.github/workflows/general.yml
index cffbcbf1f..48215e8e1 100644
--- a/.github/workflows/general.yml
+++ b/.github/workflows/general.yml
@@ -34,8 +34,6 @@ jobs:
 
       - name: Setup Dart
         uses: dart-lang/setup-dart@v1
-        with:
-          sdk: 3.6.2 # downgrade pending https://github.com/dart-lang/dartdoc/issues/3996
 
       - name: Setup Node
         uses: actions/setup-node@v4
diff --git a/doc/user_guide/_docs/A15-fsm.md b/doc/user_guide/_docs/A15-fsm.md
index ecba06b84..256ba9abe 100644
--- a/doc/user_guide/_docs/A15-fsm.md
+++ b/doc/user_guide/_docs/A15-fsm.md
@@ -113,6 +113,55 @@ FiniteStateMachine<LightStates>(
 );
 ```
 
+Notice, that in all states, one of the lights is red.  We could simplify the FSM by removing all the actions to make a light red and setting up a default in `setupActions` for the FSM.  That way, every state would default to both lights being red, and we'd only need to specify states when they are non-red.  For example:
+
+```dart
+final states = <State<LightStates>>[
+  State(LightStates.northFlowing, events: {
+    TrafficPresence.isEastActive(traffic): LightStates.northSlowing,
+  }, actions: [
+    northLight < LightColor.green.value,
+  ]),
+  State(
+    LightStates.northSlowing,
+    events: {},
+    defaultNextState: LightStates.eastFlowing,
+    actions: [
+      northLight < LightColor.yellow.value,
+    ],
+  ),
+  State(
+    LightStates.eastFlowing,
+    events: {
+      TrafficPresence.isNorthActive(traffic): LightStates.eastSlowing,
+    },
+    actions: [
+      eastLight < LightColor.green.value,
+    ],
+  ),
+  State(
+    LightStates.eastSlowing,
+    events: {},
+    defaultNextState: LightStates.northFlowing,
+    actions: [
+      eastLight < LightColor.yellow.value,
+    ],
+  ),
+];
+
+FiniteStateMachine<LightStates>(
+  clk,
+  reset,
+  LightStates.northFlowing,
+  states,
+  setupActions: [
+    // by default, lights should be red
+    northLight < LightColor.red.value,
+    eastLight < LightColor.red.value,
+  ],
+);
+```
+
 This state machine is now functional and synthesizable into SystemVerilog!
 
 You can even generate a mermaid diagram for the state machine using the [`generateDiagram`](https://intel.github.io/rohd/rohd/FiniteStateMachine/generateDiagram.html) API.
diff --git a/lib/src/finite_state_machine.dart b/lib/src/finite_state_machine.dart
index 4bbd8baa1..689f7fabc 100644
--- a/lib/src/finite_state_machine.dart
+++ b/lib/src/finite_state_machine.dart
@@ -46,6 +46,11 @@ class FiniteStateMachine<StateIdentifier> {
     return _stateValueLookup[_stateLookup[id]];
   }
 
+  /// A [Map] from the [StateIdentifier]s to the internal index used to
+  /// represent that state in the state machine.
+  late final Map<StateIdentifier, int> stateIndexLookup = UnmodifiableMapView(
+      _stateValueLookup.map((key, value) => MapEntry(key.identifier, value)));
+
   /// The clock signal to the FSM (when only single-triggered). Otherwise, the
   /// first clock.
   ///
@@ -68,6 +73,11 @@ class FiniteStateMachine<StateIdentifier> {
   /// bus.
   final Logic currentState;
 
+  /// A [List] of [Conditional] actions to perform at the beginning of the
+  /// evaluation of actions for the [FiniteStateMachine].  This is useful for
+  /// things like setting up default values for signals across all states.
+  final List<Conditional> setupActions;
+
   /// The next state of the FSM.
   ///
   /// Use [getStateIndex] to map from a [StateIdentifier] to the value on this
@@ -78,7 +88,7 @@ class FiniteStateMachine<StateIdentifier> {
   static int _logBase(num x, num base) => (log(x) / log(base)).ceil();
 
   /// Width of the state.
-  final int _stateWidth;
+  final int stateWidth;
 
   /// If `true`, the [reset] signal is asynchronous.
   final bool asyncReset;
@@ -92,7 +102,9 @@ class FiniteStateMachine<StateIdentifier> {
     StateIdentifier resetState,
     List<State<StateIdentifier>> states, {
     bool asyncReset = false,
-  }) : this.multi([clk], reset, resetState, states, asyncReset: asyncReset);
+    List<Conditional> setupActions = const [],
+  }) : this.multi([clk], reset, resetState, states,
+            asyncReset: asyncReset, setupActions: setupActions);
 
   /// Creates an finite state machine for the specified list of [_states], with
   /// an initial state of [resetState] (when [reset] is high) and transitions on
@@ -100,9 +112,14 @@ class FiniteStateMachine<StateIdentifier> {
   ///
   /// If [asyncReset] is `true`, the [reset] signal is asynchronous.
   FiniteStateMachine.multi(
-      this._clks, this.reset, this.resetState, this._states,
-      {this.asyncReset = false})
-      : _stateWidth = _logBase(_states.length, 2),
+    this._clks,
+    this.reset,
+    this.resetState,
+    this._states, {
+    this.asyncReset = false,
+    List<Conditional> setupActions = const [],
+  })  : setupActions = List.unmodifiable(setupActions),
+        stateWidth = _logBase(_states.length, 2),
         currentState =
             Logic(name: 'currentState', width: _logBase(_states.length, 2)),
         nextState =
@@ -116,26 +133,29 @@ class FiniteStateMachine<StateIdentifier> {
     }
 
     Combinational([
+      ...setupActions,
       Case(
           currentState,
           _states
               .map((state) => CaseItem(
-                      Const(_stateValueLookup[state], width: _stateWidth), [
-                    ...state.actions,
-                    Case(
-                        Const(1),
-                        state.events.entries
-                            .map((entry) => CaseItem(entry.key, [
-                                  nextState <
-                                      _stateValueLookup[
-                                          _stateLookup[entry.value]]
-                                ]))
-                            .toList(growable: false),
-                        conditionalType: state.conditionalType,
-                        defaultItem: [
-                          nextState < getStateIndex(state.defaultNextState),
-                        ])
-                  ]))
+                      Const(_stateValueLookup[state], width: stateWidth)
+                          .named(state.identifier.toString()),
+                      [
+                        ...state.actions,
+                        Case(
+                            Const(1),
+                            state.events.entries
+                                .map((entry) => CaseItem(entry.key, [
+                                      nextState <
+                                          _stateValueLookup[
+                                              _stateLookup[entry.value]]
+                                    ]))
+                                .toList(growable: false),
+                            conditionalType: state.conditionalType,
+                            defaultItem: [
+                              nextState < getStateIndex(state.defaultNextState),
+                            ])
+                      ]))
               .toList(growable: false),
           conditionalType: ConditionalType.unique,
           defaultItem: [
diff --git a/lib/src/utilities/simcompare.dart b/lib/src/utilities/simcompare.dart
index 594fe0950..771880f1b 100644
--- a/lib/src/utilities/simcompare.dart
+++ b/lib/src/utilities/simcompare.dart
@@ -160,7 +160,7 @@ abstract class SimCompare {
     }
 
     for (final vector in vectors) {
-      Simulator.registerAction(timestamp, () {
+      Simulator.registerAction(timestamp, () async {
         for (final signalName in vector.inputValues.keys) {
           final value = vector.inputValues[signalName];
           (module.tryInput(signalName) ?? getIoInputDriver(signalName))
@@ -168,7 +168,7 @@ abstract class SimCompare {
         }
 
         if (enableChecking) {
-          Simulator.postTick.first.then((value) {
+          unawaited(Simulator.postTick.first.then((value) {
             for (final signalName in vector.expectedOutputValues.keys) {
               final value = vector.expectedOutputValues[signalName];
               final o =
@@ -207,7 +207,7 @@ abstract class SimCompare {
             (Object err, StackTrace stackTrace) {
               Simulator.throwException(err as Exception, stackTrace);
             },
-          );
+          ));
         }
       });
       timestamp += Vector._period;


Test patch:
diff --git a/test/fsm_test.dart b/test/fsm_test.dart
index 0d84ed270..b5f010a56 100644
--- a/test/fsm_test.dart
+++ b/test/fsm_test.dart
@@ -122,7 +122,6 @@ class TrafficTestModule extends Module {
         TrafficPresence.isEastActive(traffic): LightStates.northSlowing,
       }, actions: [
         northLight < LightColor.green.value,
-        eastLight < LightColor.red.value,
       ]),
       State(
         LightStates.northSlowing,
@@ -130,7 +129,6 @@ class TrafficTestModule extends Module {
         defaultNextState: LightStates.eastFlowing,
         actions: [
           northLight < LightColor.yellow.value,
-          eastLight < LightColor.red.value,
         ],
       ),
       State(
@@ -139,7 +137,6 @@ class TrafficTestModule extends Module {
           TrafficPresence.isNorthActive(traffic): LightStates.eastSlowing,
         },
         actions: [
-          northLight < LightColor.red.value,
           eastLight < LightColor.green.value,
         ],
       ),
@@ -148,7 +145,6 @@ class TrafficTestModule extends Module {
         events: {},
         defaultNextState: LightStates.northFlowing,
         actions: [
-          northLight < LightColor.red.value,
           eastLight < LightColor.yellow.value,
         ],
       ),
@@ -159,6 +155,11 @@ class TrafficTestModule extends Module {
       reset,
       LightStates.northFlowing,
       states,
+      setupActions: [
+        // by default, lights should be red
+        northLight < LightColor.red.value,
+        eastLight < LightColor.red.value,
+      ],
     );
 
     if (!kIsWeb) {
@@ -179,23 +180,49 @@ void main() {
   });
 
   test('zero-out receivers in default case', () async {
-    final pipem = TestModule(Logic(), Logic(), Logic());
-    await pipem.build();
+    final mod = TestModule(Logic(), Logic(), Logic());
+    await mod.build();
 
-    final sv = pipem.generateSynth();
+    final sv = mod.generateSynth();
 
     expect(sv, contains("b = 1'h0;"));
   });
 
   test('conditional type is used', () async {
-    final pipem = TestModule(Logic(), Logic(), Logic());
-    await pipem.build();
+    final mod = TestModule(Logic(), Logic(), Logic());
+    await mod.build();
 
-    final sv = pipem.generateSynth();
+    final sv = mod.generateSynth();
 
     expect(sv, contains('priority case'));
   });
 
+  test('label name included in generated SV', () async {
+    final mod = TestModule(Logic(), Logic(), Logic());
+    await mod.build();
+
+    final sv = mod.generateSynth();
+
+    expect(sv, contains('MyStates_state1 : begin'));
+  });
+
+  test('state value lookup is correct', () async {
+    final mod = DefaultStateFsmMod(Logic());
+    await mod.build();
+
+    expect(mod._fsm.stateWidth, 2);
+
+    expect(mod._fsm.stateIndexLookup.length, MyStates.values.length);
+    expect(
+        MyStates.values.every((e) => mod._fsm.stateIndexLookup.containsKey(e)),
+        isTrue);
+    for (var i = 0; i < MyStates.values.length; i++) {
+      final stateEnum = MyStates.values[i];
+      expect(mod._fsm.getStateIndex(stateEnum), i);
+      expect(mod._fsm.stateIndexLookup[stateEnum], i);
+    }
+  });
+
   group('fsm validation', () {
     test('duplicate state identifiers throws exception', () {
       expect(
@@ -231,9 +258,9 @@ void main() {
 
   group('simcompare', () {
     test('simple fsm', () async {
-      final pipem = TestModule(Logic(), Logic(), Logic());
+      final mod = TestModule(Logic(), Logic(), Logic());
 
-      await pipem.build();
+      await mod.build();
 
       final vectors = [
         Vector({'reset': 1, 'a': 0, 'c': 0}, {}),
@@ -241,32 +268,32 @@ void main() {
         Vector({}, {'b': 1}),
         Vector({'c': 1}, {'b': 0}),
       ];
-      await SimCompare.checkFunctionalVector(pipem, vectors);
-      SimCompare.checkIverilogVector(pipem, vectors);
+      await SimCompare.checkFunctionalVector(mod, vectors);
+      SimCompare.checkIverilogVector(mod, vectors);
 
       verifyMermaidStateDiagram(_simpleFSMPath);
     });
 
     test('simple fsm async reset', () async {
-      final pipem =
+      final mod =
           TestModule(Logic(), Logic(), Logic(), testingAsyncReset: true);
 
-      await pipem.build();
+      await mod.build();
 
       final vectors = [
         Vector({'reset': 0, 'a': 0, 'c': 0}, {}),
         Vector({'reset': 1}, {'b': 0}),
       ];
-      await SimCompare.checkFunctionalVector(pipem, vectors);
-      SimCompare.checkIverilogVector(pipem, vectors);
+      await SimCompare.checkFunctionalVector(mod, vectors);
+      SimCompare.checkIverilogVector(mod, vectors);
 
       verifyMermaidStateDiagram(_simpleFSMPath);
     });
 
     test('default next state fsm', () async {
-      final pipem = DefaultStateFsmMod(Logic());
+      final mod = DefaultStateFsmMod(Logic());
 
-      await pipem.build();
+      await mod.build();
 
       final vectors = [
         Vector({'reset': 1}, {}),
@@ -275,12 +302,12 @@ void main() {
         Vector({'reset': 0}, {'b': 4}),
         Vector({'reset': 0}, {'b': 4}),
       ];
-      await SimCompare.checkFunctionalVector(pipem, vectors);
-      SimCompare.checkIverilogVector(pipem, vectors);
+      await SimCompare.checkFunctionalVector(mod, vectors);
+      SimCompare.checkIverilogVector(mod, vectors);
 
       if (!kIsWeb) {
         const fsmPath = '$_tmpDir/default_next_state_fsm.md';
-        pipem._fsm.generateDiagram(outputPath: fsmPath);
+        mod._fsm.generateDiagram(outputPath: fsmPath);
 
         final mermaid = File(fsmPath).readAsStringSync();
         expect(mermaid, contains('state2'));
@@ -293,8 +320,8 @@ void main() {
     });
 
     test('traffic light fsm', () async {
-      final pipem = TrafficTestModule(Logic(width: 2), Logic());
-      await pipem.build();
+      final mod = TrafficTestModule(Logic(width: 2), Logic());
+      await mod.build();
 
       final vectors = [
         Vector({'reset': 1, 'traffic': 00}, {}),
@@ -315,8 +342,8 @@ void main() {
           'eastLight': LightColor.green.value
         })
       ];
-      await SimCompare.checkFunctionalVector(pipem, vectors);
-      SimCompare.checkIverilogVector(pipem, vectors);
+      await SimCompare.checkFunctionalVector(mod, vectors);
+      SimCompare.checkIverilogVector(mod, vectors);
 
       verifyMermaidStateDiagram(_trafficFSMPath);
     });
diff --git a/test/logic_structure_test.dart b/test/logic_structure_test.dart
index 6947b0957..e0fbba3e5 100644
--- a/test/logic_structure_test.dart
+++ b/test/logic_structure_test.dart
@@ -93,7 +93,6 @@ class StructModuleWithInstrumentation extends Module {
       ..isOutput
       ..changed
       ..glitch
-      ..nextChanged
       // ignore: deprecated_member_use_from_same_package
       ..hasValidValue()
       // ignore: deprecated_member_use_from_same_package
@@ -102,6 +101,8 @@ class StructModuleWithInstrumentation extends Module {
       ..valueBigInt
       // ignore: deprecated_member_use_from_same_package
       ..valueInt;
+
+    unawaited(MyStruct().nextChanged);
   }
 }
 
diff --git a/test/simulator_test.dart b/test/simulator_test.dart
index 222f1ab56..f30529334 100644
--- a/test/simulator_test.dart
+++ b/test/simulator_test.dart
@@ -377,10 +377,10 @@ void main() {
   test('end of tick makes a re-tick if it was missed', () async {
     var endOfTickActionTaken = false;
 
-    Simulator.registerAction(100, () {
-      Simulator.postTick.first.then((_) {
+    Simulator.registerAction(100, () async {
+      unawaited(Simulator.postTick.first.then((_) {
         Simulator.injectEndOfTickAction(() => endOfTickActionTaken = true);
-      });
+      }));
     });
 
     var numStartTicks = 0;