Add Product Store Component to enable save/restore for a selection of data products by owhite7128 · Pull Request #196 · lasp/adamant

owhite7128 · 2026-07-30T19:01:00Z

This component adds the functionality to save a set of data products to specified memory region. It has the ability to restore data products with different set times depending on the desired behavior.

dinkelk · 2026-07-31T18:35:47Z

+      Pkt : Packet.T;
+      Stat : constant Serialization_Status := Self.Packets.Stored_Products (Self.Sys_Time_T_Get, Self.Bytes.all (Self.Bytes.all'First .. Self.Store_Last), Pkt);
+   begin
+      -- This should never fail since both the autocoder and an assertion at Init


is there such an assertion at Init?

dinkelk · 2026-08-07T13:20:34Z

+    type: Crc_16.Crc_16_Type
+    format: U16
+    skip_validation: True
+    byte_image: True


I think i'd rather this live with src/util/crc/. One less dir to add to the path for build performance, and it makes some sense to cluster it with the utility.

dinkelk · 2026-08-07T13:23:14Z

+            # (i.e. a matching recomputation). Requiring always valid types
+            # guarantees a restore can never inject a data product whose use
+            # downstream raises a constraint error:
+            if not entry.data_product.type_model.is_always_valid():


This is good. We should belt and suspenders it though. Add a compile time error to the generated Ada package which prevents compilation if somehow a possibly invalid type sneaks in.

dinkelk · 2026-08-07T13:39:07Z

-               -- The data product could not be saved and the existing slot contents
-               -- are not trustworthy, so zero the slot:
+            else
+               -- The data product could not be saved, so zero the slot:


What is the logic behind this? Intuitively to me, if we have issue fetching the data product, we should do nothing. Leave whatever the last value that was good was from the store. Overwriting with zeros could mean anything, and seems better to keep the old value.

dinkelk · 2026-08-07T13:48:54Z

+      Idx := @ + Time_Length;

      -- Restore each data product entry:
      for Item of Self.Store_Description.Entries.all loop


Unless I am missing something, I think we have an issue here. What if a data product was never stored. Whatever is in MRAM should not be restored to the database. Ideally, we would do nothing in that case, and leave the data product as Not_Available in the database.

The issue is that I think we would need some meta data to indicate that a DP is written, which doesn't currently exist. In the store you could include a byte for each DP that you write a zero or nonzero value for to indicate if the DP has been saved or never saved. We can trust that this store is not garbage if we CRC over it, so that if the CRC is good, we know this meta data store is meaningfull. Let's chat before you implement this. There could be better ideas.

dinkelk · 2026-08-07T14:19:44Z

+         end if;
+         -- An entry stores its data product's timestamp if and only if that
+         -- timestamp is what the entry is restored with:
+         pragma Assert (Item.Store_Timestamp = (Item.Restore_Time = Use_Stored_Dp_Time));


If this is true, do we need Store_Timestamp boolean at all? Could we just use Restore_Time to do the logic we need?

Maybe we want this, just in case we want to store the DP timestamp for dump. Weird use case, but maybe OK.

dinkelk · 2026-08-07T14:29:04Z

+   begin
+      -- This should never fail since the autocoder and the Store_Size_Type
+      -- constraint guarantee that the store fits within a single packet:
+      pragma Assert (Stat = Success);


I still don't see an assertion Init that proves this comment. Where is it?

dinkelk · 2026-08-07T14:31:27Z

+      end if;
+
+      -- Increment the tick counter, rolling over at Ticks_Per_Save:
+      Self.Tick_Count := (@ + 1) mod Self.Ticks_Per_Save;


Seems like this should be gated by Save_On_Tick to be more correct and save computation if Save_On_Tick is False.

dinkelk · 2026-08-07T14:33:19Z

+      use Command_Execution_Status;
+   begin
+      Self.Save_On_Tick := True;
+      Self.Event_T_Send_If_Connected (Self.Events.Save_On_Tick_Enabled (Self.Sys_Time_T_Get));


Seems like we should reset count related to this here.

dinkelk · 2026-08-07T15:06:54Z

One somewhat large design issue that is not addressed:

No atomicity across a save. Do_Save overwrites the store in place and writes the CRC last. A reboot mid-save corrupts the entire store, so the Set_Up restore fails and all products are lost, not just the ones being written. Power loss during a write is fairly central to this component's mission. The CRC-last ordering means it fails safe, which is good, but a double-buffered layout (two copies, alternate writes, restore the valid/newer one) would survive it.

We should discuss this as an option. AI suggests:

Ping-pong with a sequence number (double buffer done properly). Worth distinguishing from a naive double buffer: two copies, alternate writes, and a monotonic save
  counter inside each CRC-protected region. Restore validates both, picks the valid copy with the newer counter. You never overwrite the only good copy, and a mid-save
  reboot costs you exactly one save interval of freshness. This is the classic EEPROM/MRAM pattern and the most robust option, at the cost of doubling the memory footprint
  and complicating the dump command (dump which copy? probably both, or the active one plus a data product reporting which is active).

Alternatively, we go to a best effort mode instead, and potentially drop the CRC altogether.

dinkelk · 2026-08-07T15:08:05Z

A few AI flagged issues worth looking at:

  2. component-product_store-implementation.ads doc drift: the Init comment block lists Bytes, Restore_On_Set_Up, and Commands_Dispatched_Per_Tick but omits Ticks_Per_Save
  (the .adb version has all four). Also the private-part connector comment still says "Each tick received saves the data products to the store," which predates the divider
  and command dispatch.
  3. Dead/hardcoded code in gen/models/stored_products.py: crc_obj = [None] is declared and never used, and _get_crc_size() hardcodes 16 bits rather than deriving it from
  the Crc_16 type. The hardcode is acknowledged in a comment and probably fine, but the dead variable should go.
  4. Unhelpful failure mode in gen/models/product_store_packets.py: in set_assembly, if the specific-name path matching finds nothing, model_path stays None and
  model_loader.load_model(None) will blow up with an obscure error. An explicit ModelException naming the model it looked for would save someone an afternoon. Related: the
  configs.split(".")[0].split("_Stored_Products") parsing is case-sensitive, but Ada is not, so a user writing the discriminant as
  TEST_ASSEMBLY_STORED_PRODUCTS.Store_Description'Access would compile fine and fail model generation confusingly.
6. No test passes a byte array larger than Store_Size or one with a nonzero 'First, both of which Init explicitly permits. The index arithmetic (Data_First, Store_Last)
  handles both correctly as written, but a test would pin that behavior against refactors.
  7. Test_Missing_Data_Product verifies the zeroed store bytes but never restores them, so the "zeros flow back into the database" behavior is untested. If the validity-byte
  redesign happens, this test becomes the natural home for "invalid slots are skipped on restore."

… data products

- Remove No_Time save time option; a save time is always written - Default restore_time to Use_Save_Time; require Use_Stored_Dp_Time when store_timestamp is True (and vice versa) - Require stored data product types to be always valid in the model - Add Enable_Save_On_Tick/Disable_Save_On_Tick commands and events - Add Save_Count, Restore_Count, Crc_Invalid_Count data products, seeded at Set_Up - Add Ticks_Per_Save init parameter to divide down the save rate; dispatch queued commands after tick business logic - Zero the slot of any data product that cannot be saved (removes prior-CRC preservation logic); make Id_Out_Of_Range an unmaskable dedicated event - Make Do_Restore a Boolean function; inline one-line helpers - Replace component-local store_crc_error with shared framework type Crc_Mismatch_Info in src/types/crc - Rename main test model to specific-named backup.test_assembly .stored_products.yaml, verifying multi-instance model support - Remove test_no_time suite (No_Time removed); add tests for seeding, enable/disable, tick divider, and id-out-of-range; 15/15 tests pass, 100% implementation coverage, style clean Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

- Add a stored length byte to each store entry (zero = never saved). Entries that cannot be fetched during a save keep their previous contents; a save onto an invalid store sanitizes it first so garbage can never carry a nonzero stored length - Silently skip never-saved entries on restore, leaving those data products unavailable in the database; refuse entries whose stored length mismatches the model (new Stored_Length_Mismatch event), which detects stored products model changes across boots - Emit compile-time Always_Valid checks in the generated table package - Move crc_mismatch_info to src/util/crc alongside the CRC utility - Gate the tick divider under Save_On_Tick and reset it on re-enable, so enabling deterministically saves on the next tick - Restate the store-fits-in-packet guarantee as an Init assertion - Tests: new suites for restore-skips-unwritten and stored-length mismatch, per-id fetch failure control in the tester, divider freeze/reset coverage; 17/17 tests pass, 100% implementation coverage, style clean Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The store previously held a single copy written in place with the CRC written last. A reboot mid-save failed safe (the restore refused the corrupted store) but lost every stored product. The store is now double buffered: each save writes the copy NOT holding the most recent valid save, stamps it with a monotonic save counter inside the CRC-protected region, and writes the CRC last. A restore validates both copies and reads from the valid copy holding the newest counter, so a mid-save reboot costs at most one save interval of freshness. - Init now takes two byte array pointers (Bytes_A/Bytes_B), each at least Store_Size bytes, so the copies can be placed in separate memory banks. - Dump_Store emits two packets (Stored_Products_A/B), one per copy, dumped as-is for ground inspection. - Products_Saved and Products_Restored events report the copy and its save counter (Store_Copy_Info.T); Store_Crc_Invalid reports a CRC mismatch per copy (Copy_Crc_Mismatch_Info.T) when a restore finds no valid copy. - Slots whose data product cannot be fetched during a save now keep the values from the most recent valid save via copy seeding. - New tests: mid-save reboot recovery (ping-pong fallback and recovery) and oversized byte arrays with nonzero first indices. - Also addresses review feedback: implementation spec doc drift, dead code in stored_products.py, and a clear ModelException (with case-insensitive package parsing) when product_store_packets cannot resolve the stored products model. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

owhite7128 force-pushed the product-store-component branch from 11c1efc to 0f89ae7 Compare July 30, 2026 19:04

owhite7128 requested a review from dinkelk July 30, 2026 19:05

dinkelk reviewed Jul 31, 2026

View reviewed changes

Comment thread src/components/product_store/product_store.component.yaml Outdated

dinkelk reviewed Jul 31, 2026

View reviewed changes

Comment thread src/components/product_store/product_store.component.yaml Outdated

dinkelk reviewed Jul 31, 2026

View reviewed changes

Comment thread src/components/product_store/product_store.component.yaml Outdated

dinkelk reviewed Jul 31, 2026

View reviewed changes

Comment thread src/components/product_store/product_store.commands.yaml

dinkelk requested changes Jul 31, 2026

View reviewed changes

dinkelk reviewed Aug 7, 2026

View reviewed changes

dinkelk requested changes Aug 7, 2026

View reviewed changes

owhite7128 and others added 4 commits August 13, 2026 13:02

Add Product Store Component to enable save/restore for a selection of…

aebad4b

… data products

owhite7128 force-pushed the product-store-component branch from 45adb97 to 78d3129 Compare August 13, 2026 19:03

Uh oh!

Conversation

owhite7128 commented Jul 30, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dinkelk commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dinkelk commented Aug 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dinkelk commented Aug 7, 2026 •

edited

Loading