-
Notifications
You must be signed in to change notification settings - Fork 133
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
support private tags and tag numbers >30 that are stored in long form #1416
base: master
Are you sure you want to change the base?
Conversation
I've made it generic for my use case. /// Application class
type Application<T, const TAG: u16> = CustomClass<T, TAG, 0b01>;
/// Context-specific class
type ContextSpecific<T, const TAG: u16> = CustomClass<T, TAG, 0b10>;
/// Private class
type Private<T, const TAG: u16> = CustomClass<T, TAG, 0b11>;
/// Application, Context-specific or Private class field which wraps an owned inner value.
///
/// This type decodes/encodes a field which is specific to a particular context
/// and is identified by a [`TagNumber`].
#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
pub struct CustomClass<T, const TAG: u16, const CLASS: u8> {
/// Tag mode: `EXPLICIT` VS `IMPLICIT`.
pub tag_mode: TagMode,
/// Value of the field.
pub value: T,
} Shouldn't |
This crate is still unusable for the European Tachograph regulation. There are many 2-byte tags. https://eur-lex.europa.eu/legal-content/EN/TXT/?uri=celex%3A02016R0799-20200226
|
@dishmaker we're still open for breaking changes in this release cycle. There is a legitimate use-case, PRs are welcome! |
Sorry for the slow reaction. In a week I am back and try to get in some more progress here. |
@kamulos I've finished making the other changes I consider blockers for this one, though it's created quite a few conflicts. If you can get this PR green though, I can make it a priority for the next to merge. I don't expect there will be many other major changes to |
@tarcieri Thanks for your work ❤️ And sorry for being very slow, the workload at the job was high and I also had vacations. Currently we are actually releasing this code into production and things will calm down from here. So I will get back to this next week and hopefully get some version ready that is good quality. |
@kamulos no worries, we'd like to do a release "soon", but that's probably in a month or two so you have some time |
a1ef847
to
39e8b97
Compare
39e8b97
to
1f43a08
Compare
@tarcieri I rebased and polished the implementation. The unit tests for the But other than that I think it is ready for review. 👍 There are some noteworthy points:
|
Well, I also need I think the best solution is using generics: pub type ApplicationExplicit<const TAG: u16, T> = CustomClassExplicit<T, TAG, CLASS_APPLICATION>;
pub type ContextSpecificExplicit<const TAG: u16, T> = CustomClassExplicit<T, TAG, CLASS_CONTEXT_SPECIFIC>;
pub type PrivateExplicit<const TAG: u16, T> = CustomClassExplicit<T, TAG, CLASS_PRIVATE>;
pub type ApplicationImplicit<const TAG: u16, T> = CustomClassImplicit<T, TAG, CLASS_APPLICATION>;
pub type ContextSpecificImplicit<const TAG: u16, T> = CustomClassImplicit<T, TAG, CLASS_CONTEXT_SPECIFIC>;
pub type PrivateImplicit<const TAG: u16, T> = CustomClassImplicit<T, TAG, CLASS_PRIVATE>;
/// EXPLICIT
#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
pub struct CustomClassExplicit<T: for<'a> Decode<'a>, const TAG: u16, const CLASS: u8> {
pub value: T,
}
/// IMPLICIT
#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
pub struct CustomClassImplicit<T: for<'a> DecodeValue<'a>, const TAG: u16, const CLASS: u8> {
pub value: T,
}
pub const CLASS_APPLICATION: u8 = 0b01000000;
pub const CLASS_CONTEXT_SPECIFIC: u8 = 0b10000000;
pub const CLASS_PRIVATE: u8 = 0b11000000; |
I've fixed all dependencies and added these 3 custom tags as generic ones :) |
closes #1381
This PR implements two things:
PRIVATE
tags be annotated like context specific ones in the derive macrosCurrent State
Currently this is a draft to get feedback on the chosen approach. This code is in use in my project and works correctly for it's use-case: a lot of private tags in a deep hierarchy of choices and sequences.
For the most part this code should be in a state to be reviewed. There are a few parts that are still lacking:
private
module, that is basically just a copy of thecontext_sensitive
moduleOverview
TagNumber
is now represented by anu16
. This should be enough for any use-case imaginable, but also small enough to be embedded-friendly. It's content is nowpub
instead ofpub(super)
. This was necessary becauseder_derive
wants to match against constantTagNumber
s. In general having this public should not be an issue, because all2^16
tag numbers are now valid and it is not possible to create an invalid state by having access to theu16
.Also the
Private
andPrivateRef
types exist now. In the derive macrosprivate
can be annotated in the same way ascontext_specific
. Internally this is represented asclass: Option<Class>
instead ofcontext_sensitive: Option<TagNumber>
.Breaking changes that I found no way to avoid are:
TagNumber::new()
andTagNumber::value()
useu16
nowu8
forTag
andTagNumber
do not make sense anymore, this also applies toTag::octet()
Reader
trait now needs a function to peek multiple bytes in advance to be able to peek a tag (Reader::peek_offset()
)Length::for_tlv()
just assumed, that a tag is always one byte, it now somehow needs to know about the specific tag we are looking at