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

Introduce an optional parameter to use non-default padding #133

Merged
merged 41 commits into from
Feb 5, 2024

Conversation

nuts4coffee
Copy link
Contributor

@nuts4coffee nuts4coffee commented Feb 1, 2024

Brief

Add an optional parameter pad_with_zero to allow use a different padding of 1 instead of the default padding 0.

Checklist

  • Add relevant labels to the Pull Request
  • Review test results and code coverage
    • Review snapshot test results for deviations
  • Review code changes
    • Create relevant test scenarios
    • Update examples
    • Update JSON schema
  • Update documentation
    • Update examples in README
  • Update changelog
  • Update version number

Resolves

Resolves #131

  • Describe the bug or feature and link to relevant issues

Evidence

Unit test updated.
Also via end-to-end test that generates the same payload data as Canoe with the signal definition in #131.

  • Analyze how the change might impact existing code
    The change is backwards compatibale and does not change the existing behavior.

  • Provide evidence that the feature is tested and covered properly

Yan Du and others added 30 commits June 1, 2023 15:08
Add support for protocol version J2602_1_1.0
Fix issue with decode/encode signals of list type
Bump version number
Encode to LSB for multi-byte array signals
Copy link

codecov bot commented Feb 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (31b4fa6) 96.30% compared to head (bef9878) 96.31%.

❗ Current head bef9878 differs from pull request most recent head f5efef7. Consider uploading reports for the commit f5efef7 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #133   +/-   ##
=======================================
  Coverage   96.30%   96.31%           
=======================================
  Files          13       13           
  Lines        1463     1465    +2     
=======================================
+ Hits         1409     1411    +2     
  Misses         54       54           
Flag Coverage Δ
3.10 96.31% <100.00%> (+<0.01%) ⬆️
3.11 96.31% <100.00%> (+<0.01%) ⬆️
3.12 96.31% <100.00%> (+<0.01%) ⬆️
3.6 96.31% <100.00%> (+<0.01%) ⬆️
3.7 96.31% <100.00%> (+<0.01%) ⬆️
3.8 96.31% <100.00%> (+<0.01%) ⬆️
3.9 96.31% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

c4deszes
c4deszes previously approved these changes Feb 2, 2024
Copy link
Owner

@c4deszes c4deszes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I'll bump the version number and release it tomorrow.

Maybe one comment is that the padding style I think is needlessly stored on the LDF object. Do you plan on using it somewhere?

@nuts4coffee
Copy link
Contributor Author

Looks good, I'll bump the version number and release it tomorrow.

Maybe one comment is that the padding style I think is needlessly stored on the LDF object. Do you plan on using it somewhere?

The padding style on the LDF object is being used when parsing the LDF: https://github.com/nuts4coffee/ldfparser/blob/padding_option/ldfparser/parser.py#L128

@nuts4coffee nuts4coffee requested a review from c4deszes February 5, 2024 13:20
@c4deszes c4deszes merged commit 2ec017f into c4deszes:master Feb 5, 2024
13 checks passed
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

Successfully merging this pull request may close these issues.

Incorrect padding of unused bytes
2 participants