PdoBase: Forbid getting item with zero index (fixes #581, #607) #609
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This was referenced Aug 5, 2025
acolomb
changed the title
The access by this index number mixed up the two number ranges. The CiA 301 standard defines: * Object 1600h to 17FFh: RPDO mapping parameter * Object 1A00h to 1BFFh: TPDO mapping parameter The test was also wrong, because it added the two tested variables to the TPDO1, while testing the __getitem__ lookup with index 0x1600.
Augment test_pdo_getitem() to not only check the mapped object values. What's more important is the type of object returned, and whether it is the correct object (identical to other access method results).
These two PdoBase-derived classes only supported numeric access based on the sequential index, not with the mapping parameter record index like the PDO class. Implement a fallback to check that if the regular index lookup fails.
The mechanisms to lookup objects, implemented in class PdoMaps, did not apply to the PDO class itself, because it simply used a dictionary instead of a PdoMaps object. That also violates the static typing rules. Make the PdoBase.maps attribute mandatory and accept only type PdoMaps. To allow a basically "empty" PdoMaps object, adjust its constructor to skip adding entries when neither offset parameter is given as non-zero. Instead, access the PdoMaps.maps attribute directly to inject the offset-based TX and RX PdoMap objects in the PDO class constructor. Add some explanation why relative indices cannot be used here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
PdoBasemapping uses an enumeration starting from one for the individual PDOs. Requesting index 0 (zero) accidentally will return the firstPdoVariablein the first PDO. This is coincidental, because the PDOs are tried in turn and with a PDO, the numeric index lookup is zero-based.To avoid hard-to-debug errors, simply forbid the value zero completely in the lookup, which does not make sense for any of the supported lookup methods.