WIP: refactor periodic HK helpers #37
Loading…
x
Reference in New Issue
Block a user
No description provided.
Delete Branch "refactor-periodic-hk-helpers"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Little helpers is what we need, not over-engineered managers..
66555301fatod2271e8185d2271e8185tof68653ba2840cf39721fto97e92c0dd255e1e0b05ctobb95797073e660cfad72tofd89c33aaeremoving valid flags is a straight up terrible idea. This PR breaks how operations works. Having to implement valid flags yourself makes it even worse, as OPS cannot be sure whether there is a vaild flag for TM or not, because it was left up to the developer. I don't even see the advantage, as this looks like you need more code now than before.
This PR should not be merged!
I could add back the valid flag for the whole set, as this is the one that is required the majority of times. I am not going to add back the valid flag for individual pool entries. It breaks the simplest case where someone just wants to download some arbitrary blob of data as downlink HK. If someone requires en explicit valid flag on an individual field, that is an explicit requirement which should be explicitly encoded, and I don't see a problem of doing that via an additional field. If someone does not know that
set.valid = trueis a thing that should be done, they'll probably not writeset.setValid()as well.If the validity of the whole set is added back, it should be serialized somewhere, and that probably would be some u8 field before the data blob. There is the additional detail of whether the HK helper does this, because the HK helper does not have a concept of validities anymore. It does the one thing that it is supposed to do, and nothing more: Track and execute periodic generation of arbitrary data blobs specified by the user. It transfers the serialization task itself to the user, but it does not currently know how the data looks and whether it is valid. Adding an explicit u8 valid field in the set itself solves this as well, because the serialization call will automatically serialize the valid field as well.
I disagree with more code being a problem. It's one small line of code (the additonal valid field). This does not break OPS either. Whats the difference between a set of all false valid flags ignored by the developer, or a set where the valid field is still missing? Both require a SW update / patch. This is something that can be solved via documentation and/or teaching basic OPS aspects to the developers..
If you think this is something that should not be merged, I'd also like to hear improvement suggestions, because I still think this PR removes unused and over-engineered cruft. Furthermore, the incraesed flexbility / lower requirements to the data format enables moving to serialization formats like
protobuf. The removal of the valid flags is controversial, but I was never happy about those weird bitfields at then end of the HK data blob either. Oftentimes, we either forgot deserializing them, and even if we deserialize them, the valid bits still need to be mapped to the set values, which is not explicit either, it needs to be implicitely known how many values the set has.This PR is all about enforcing the KISS principle and getting rid of unnecessary complexity from the basic building blocks used. If the small price is adding a few fields (which I already have done for all SOURCE datasets), it's a price I am willing to pay.
The validity state and the validity blobs can now be used again by exposing additional appropriate API for the shared sets again, or by using new dedicated HK specific types and APIs for non-shared sets again.
cef9291737toc3898b3928Summary of the most important changes:
6bafafddd6toa46a66c35areplaced by #45
Pull request closed