Skip to content

eds: fix LowLimit/HighLimit parsing for all signed integer types#658

Open
friederschueler wants to merge 1 commit intocanopen-python:masterfrom
friederschueler:fix/eds-limit-parsing
Open

eds: fix LowLimit/HighLimit parsing for all signed integer types#658
friederschueler wants to merge 1 commit intocanopen-python:masterfrom
friederschueler:fix/eds-limit-parsing

Conversation

@friederschueler
Copy link
Copy Markdown
Collaborator

Fixes incomplete signed integer handling for LowLimit/HighLimit in EDS parsing, as noted in #352.

Changes

  • Replace _calc_bit_length() (only handled INTEGER8/16/32/64) with a lookup dict _SIGNED_BIT_LENGTHS covering all 8 SIGNED_TYPES (INTEGER8/16/24/32/40/48/56/64)
  • Log a warning instead of silently ignoring invalid limit values (except ValueError: pass)
  • Add EDS test entries in sample.eds for INTEGER24/40/48/56 with hex-encoded negative limits
  • Extend test_record_with_limits and add test_invalid_limit_logs_warning

Note on formatting

eds.py and test_eds.py were reformatted with black. If a purely functional diff is preferred, the formatting commits can be dropped — the functional changes are small and easy to separate.

Closes part of #352.

- Replace incomplete _calc_bit_length() (only handled 4 of 8 signed types)
  with a lookup dict covering all SIGNED_TYPES (INTEGER8/16/24/32/40/48/56/64)
- Log a warning instead of silently ignoring invalid limit values
- Add EDS test entries for INTEGER24/40/48/56 with hex-encoded negative limits
- Extend test_record_with_limits and add test_invalid_limit_logs_warning

Note: eds.py and test_eds.py were formatted with black. The formatting
changes can be dropped if a pure functional change is preferred.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 6, 2026

Codecov Report

❌ Patch coverage is 81.25000% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
canopen/objectdictionary/eds.py 81.25% 4 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

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.

1 participant