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

Physical value encoder rounding #142

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

c4deszes
Copy link
Owner

Brief

  • Previously the physical values used int(x) to round the values, which rounds down
  • The new one should be more consistent across input ranges

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

(Issue filed externally)

Evidence

  • This change might break user code where users have not fully made sure to limit the input and output ranges, for example a signal intended to be encoded into 0-254 range if the user input could be as high 254.8 then this could lead to an error of the signal overflowing into 255. But at least the encoder will catch cases if that would go outside it's range

@c4deszes c4deszes added ldf/encoding Related to signal encoding priority:normal Normal priority labels Jan 23, 2025
Copy link

codecov bot commented Jan 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.31%. Comparing base (fbf34fe) to head (801a7e6).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #142   +/-   ##
=======================================
  Coverage   96.31%   96.31%           
=======================================
  Files          13       13           
  Lines        1467     1467           
=======================================
  Hits         1413     1413           
  Misses         54       54           
Flag Coverage Δ
3.10 96.31% <100.00%> (ø)
3.11 96.31% <100.00%> (ø)
3.12 ?
3.6 96.31% <100.00%> (ø)
3.7 96.31% <100.00%> (ø)
3.8 96.31% <100.00%> (ø)
3.9 96.31% <100.00%> (ø)

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ldf/encoding Related to signal encoding priority:normal Normal priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant