Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: API to set a range of bits using offset and length #30

Open
rnestler opened this issue Oct 26, 2020 · 3 comments
Open

feature: API to set a range of bits using offset and length #30

rnestler opened this issue Oct 26, 2020 · 3 comments

Comments

@rnestler
Copy link

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-8ba2aa3932cb35175d51de283952144c82bcf45eef54f18f72f199650929ee80R24

It would have been more intuitive for me to pass a bit_offset and a bit_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? 😉

@dzamlo
Copy link
Owner

dzamlo commented Oct 26, 2020

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 ?

@rnestler
Copy link
Author

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 something
  • set_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?

@dzamlo
Copy link
Owner

dzamlo commented Oct 27, 2020

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.

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

No branches or pull requests

2 participants