Open rnestler opened 4 years ago
The BitRange
trait was never really intended to be used as-is. The goal of this trait is the method can be called from the code generated by the macro, not by a developer. That explain the" quite a weird API".
But adding such a method would be quite simple. The hardest thing will be finding a name for this methods. Any suggestions ?
The goal of this trait is the method can be called from the code generated by the macro, not by a developer.
So I'm using bitfield
wrong / not as intended? Is there something already existing I could use instead?
But adding such a method would be quite simple. The hardest thing will be finding a name for this methods. Any suggestions?
Finding a good name is indeed hard here.
A few ideas:
insert_bits(bit_index, number_of_bits, value)
Makes it clear that we insert a number of bits at a position. But may wrongly imply that we shift stuff around or somethingset_bits_at(bit_index, number_of_bits, value)
append_bit_range(bit_index, number_of_bits, value)
Maybe less confusing than insert?set_bit_range_at(bit_index, number_of_bits, value)
Similar name to the existing one.Also maybe there is a better name than bit_index
?
So I'm using bitfield wrong / not as intended?
The intended use of this crates is really to use the bitfield!
macro with defined fields, and then you access the fields with the generated methods.
The current implementation of BitRange
(and now BitRangeMut
on master) is quite naive and is only fast when LLVM can do a good job. This depends heavily on the fact that the parameters are constant, last time I tested.
I didn't try it, but bitvec
maybe more appropriate for your use case.
But if you find that this crates works better for you and adding that method is useful for you, I can do that.
I recently used
bitfield
in a project where I found the API passing MSB and LSB a bit cumbersome: https://github.com/gfroerli/firmware/pull/82/files#diff-8ba2aa3932cb35175d51de283952144c82bcf45eef54f18f72f199650929ee80R24It would have been more intuitive for me to pass a
bit_offset
and abit_length
: Like that:output.set_bit_range(*bit_index, Self::SIZE, self.0);
Would you consider adding such an API to this crate or do you think the idea is rubbish? :wink: