feature/length-padding

[simylein]

This is my first attempt at implementing #13
I am particularly unsure if I am supposed to pass command line args like that.
And yes, I know I called it --auto-paddding instead of --skip-length-validation-and-pad.
I think the latter is just too long as a command line flag (also having trouble finding a short version of it)
So therefore I propose --auto-padding or --payload-padding or something similar.
However, if you insist on your flag name I will gladly add another commit renaming it.
Please let me know if this is the right approach, or if you would like to have it done differently (coding wise).

[michaelbeutler]

I would still suggest the flag containing some kind of trigger word like "skip" or "unsafe". This flag is only a workaround for very unique projects.

[michaelbeutler]

Please add more tests to keep the test coverage.

[simylein]

We kinda have a problem, because there are optional fields.
Consider a message of 12 bytes which uses a port config on which the first 8 bytes are required and a further 8 bytes optional.
Now if the message is 11 bytes long there is no way of knowing how many zeros to add because it could be just 1 byte of zeros or also 5 bytes of zeros.
We can just say to add zeros until the nearest full byte, but then your example of 123 -> 00000123 is impossible as we need to add 2 bytes and 4 bits for which we need to know that the message needs to be 4 bytes long.
I am not really sure how to proceed, I can just add test cases which check for scrambled data, or only test port implementations which do not have any optional fields.

[michaelbeutler]

Since this is an workaround, we don't expect this to happen and no optional fields will be set when this flag is active.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines	Coverage Δ
pkg/decoder/helpers/helpers.go	93.22% <100.00%> (+0.91%)	⬆️
pkg/decoder/nomadxl/v1/decoder.go	95.16% <100.00%> (+1.16%)	⬆️
pkg/decoder/nomadxs/v1/decoder.go	87.09% <100.00%> (+1.91%)	⬆️
pkg/decoder/tagsl/v1/decoder.go	98.07% <100.00%> (+0.07%)	⬆️
pkg/decoder/tagxl/v1/decoder.go	92.45% <100.00%> (+2.70%)	⬆️

[simylein]

Please add more tests to keep the test coverage.

Has been done, thanks for the reminder 🙂

[simylein]

I would still suggest the flag containing some kind of trigger word like "skip" or "unsafe". This flag is only a workaround for very unique projects.

What do we do about the naming situation? Would you like me to rename to --skip-length-validation-and-pad?
After this is resolved, this pull request would be ready to merge.

[michaelbeutler]

We can go with auto-padding but should remove the shortcut -p. Additionally, we should consider using the Functional Options Pattern to minimize interface changes.
See example: https://dev.to/kittipat1413/understanding-the-options-pattern-in-go-390c

type Option func(*Decoder)

func WithAutoPadding() Option {
    return func(d *Decoder) {
        d.autoPadding = true
    }
}

[simylein]

We can go with auto-padding but should remove the shortcut -p. Additionally, we should consider using the Functional Options Pattern to minimize interface changes. See example: https://dev.to/kittipat1413/understanding-the-options-pattern-in-go-390c

type Option func(*Decoder)

func WithAutoPadding() Option {
    return func(d *Decoder) {
        d.autoPadding = true
    }
}

I would need some guidance with that as I looked into it and couldn't quite figure out how to apply this in the code base.

[simylein]

thank you for implementing the functional options