Skip to content

Comments

Try new multi-register implementation#184

Draft
diondokter wants to merge 7 commits intomasterfrom
multi-register-test2
Draft

Try new multi-register implementation#184
diondokter wants to merge 7 commits intomasterfrom
multi-register-test2

Conversation

@diondokter
Copy link
Owner

@diondokter diondokter commented Jan 4, 2026

One of the biggest issues that remained unsolved in #38 was about the allocation of the fieldsets.
Const generics is not yet ready to allocate a buffer based on the sum of multiple generic consts.

So then the idea was that we could let the user allocate a buffer and then we'd allocate the fieldsets in there.
But that sucks. It'd require having non-owned variants of all fieldsets.

After thinking more I realized that was not the case. Fieldsets are just an array of bytes. If we make the fieldsets repr transparent, then we can split slices into &mut [u8; N] which we can then transmute into a mutable reference to the fieldset.

I still couldn't make this work. But then another realization hit. In memory, when two arrays are next to each other without padding inbetween, you can legally consider them 1 big array together.
So I started looking at the layout of tuples. The idea being that if the layout is guaranteed, you could cast a (FooFieldSet, BarFieldSet) to a u8 slice.

Sadly, tuples are repr(rust) and have no stable representation. But that's not the case for tuple structs. You can mark those repr(C)!
With the C ABI, the order is preserved. And all members are align(1) so no padding is added. Hence it's safe to get a u8 slice to it.
This means the allocation can be done by the compiler instead of by the user. :D

This leads to the following API:

device
.multi_write()
.with(|d| d.bar(0).plan_with_zero())
.with(|d| d.foo().plan())
.execute(|(bar, _foo)| {
bar.set_value(42);
})
.unwrap();
let multi = device
.multi_read()
// TODO: Allow reading multiple. This would return a [Bar;N] that can also impl FieldSet.
// Maybe N is a const generic on foo_repeated with default 1?
// We'll also need a check whether it's allowed by the device rules. That's probably an assert.
.with(|d| d.bar(0).plan())
.with(|d| d.foo().plan())
.execute()
.unwrap();
assert_eq!(
multi,
(
BarFieldSet::from([42]),
FooFieldSet::from([0xFF, 0xFF, 0xFF])
)
);
device
.multi_modify()
.with(|d| d.bar(0).plan())
.with(|d| d.foo().plan())
.execute(|(bar, foo)| {
bar.set_value(-5);
foo.set_value_0(false);
})
.unwrap();
let multi = device
.multi_read()
.with(|d| d.bar(0).plan())
.with(|d| d.foo().plan())
.execute()
.unwrap();
assert_eq!(
multi,
(
BarFieldSet::from([-5i8 as u8]),
FooFieldSet::from([0xFE, 0xFF, 0xFF])
)
);

This PR does not support mixing reads and writes in one transaction. I've decided that's a different issue entirely (even if it's got overlap) and this PR doesn't preclude that functionality.


TODO:

  • Clean up. Move Device trait into main crate and change templates to impl it on devices and blocks. Consider renaming it to Block
  • Consider doing the multi_write the same way as multi_modify where there's one callback in the execute function instead of one per plan. This would allow not having to call the plan functions on the register operations. The only thing is that we still need to support the write_with_zero usecase, so there may still need to be a .plan function for that.
  • Add tests
  • Add rules and checks
  • Add the ability to do multi-ops on repeated registers that use array types. See the todo comment in the code
  • Implement fully packing the fieldsets (FieldSetArray & FsSet)
  • Fix docs
  • Check binary size impact and do some optimizations
  • Add support for non-multiple-of-8-bits fieldsets. This requires packing and unpacking the bits. This is slow, but probably only for non-multiple-of-8-bits fieldsets. Or maybe it's too difficult and we don't support it.
  • Make sure this is really the API we want. Also think about Devices need reserved names #183 since that's relevant.

@diondokter
Copy link
Owner Author

Oof, rough with the generics!
But it's working :D

Need to check binary size. I'm banking a lot on the optimizer here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant