Fix and extend PDO selection by record IDs (fixes #607) #613
+59
−15
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 is currently based on, and includes the changes from #609, after basically factoring out the unrelated fixes from that PR. |
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.
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.
This does not work for the generic, joined accessor class PDO yet.
Store each PDO under two different index numbers in the dummy PdoMaps object's "maps" attribute. That would duplicate them in sequential access while iterating / counting though, so override the PdoBase dunder methods to explicitly chain just the two sub-sequences rx and tx.
The base iterators of rx and tx return a 1-based sequence, while the wrapping PDO class allows only mapping or communication record index numbers to identify a PdoMap object. Return the mapping record's index from the iterator to avoid ambiguity. Add tests to verify the returned objects are actually the RPDO and TPDO members (in this order), with strictly increasing index numbers.
acolomb
force-pushed
the
pdo-getitem-access-patterns
branch
from
2635b3f to
7e67d90
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fix mixed up mapping parameter record index numbers in class PDO. The access by this index number mixed up the two number ranges. The CiA 301 standard defines:
The test was also wrong, because it added the two tested variables to the TPDO1, while testing the
__getitem__lookup with index 0x1600.Support lookup by mapping record and communication parameter record index in
TPDOandRPDOclasses, by storing record index offsets for eachPdoMapscollection. These twoPdoBase-derived classes only supported numeric access based on the sequential index, not with the mapping parameter record index like thePDOclass. Implement a fallback to check those if the regular index lookup fails. Fix docstrings to cover whole RPDO / TPDO parameter index ranges.Support lookup by communication parameter record index in generic
PDOclass, by storing each PDO under two different index numbers in the dummyPdoMapsobject'smapsattribute. That would duplicate them in sequential access while iterating / counting though, so override thePdoBasedunder methods to explicitly chain just the two sub-sequencesrxandtx. This class now uses a properPdoMapsproxy object as well, instead of overwriting it with a custom dictionary. To allow this dummy usage, adjust its constructor to skip generating entries. Add some explanation why relative indices cannot be used here.