Skip to content

Can't access T/RPDO via ID #607

Closed

Description

@maxwai

The __getitem__ implementation for PdoBase has following check:

        if isinstance(key, int) and (0x1A00 <= key <= 0x1BFF or   # By TPDO ID (512)
                                     0x1600 <= key <= 0x17FF or   # By RPDO ID (512)
                                     0 < key <= 512):             # By PDO Index
            return self.map[key]

Suggesting that one could get the PDO with the Mapping Parameter Index

But looking at the __getitem__ of PdoMaps:

    def __getitem__(self, key: int) -> PdoMap:
        return self.maps[key]

one can see that it makes a normal dictionary fetch.

When looking at how the dict is populated in the __init__:

        self.maps: Dict[int, PdoMap] = {}
        for map_no in range(512):
            ...
                self.maps[map_no + 1] = new_map

you see that the key is a normal index counting up the PdoMap added. So an access by the Mapping Parameter isn't possible.

Additionally I think an access via the Communication Parameter in addition to the Mapping Parameter could make sense. Either way, either the check in the PdoBase needs to be redacted to only check 0 < key <= 512 or (what I think makes more sense) the __getitem__ in PdoMaps needs to be changed.

Activity

  1. acolomb commented on Aug 1, 2025

    @acolomb
    Member

    This was previously reported in #581 and a (partial) fix is up for review in #602. Please chime in there, whether the approach would suit you.

    Looking at the code again, we really do seem to be missing the actual code to support looking up by parameter object index (0x1400, 0x1600, etc.). Even though it doesn't work right now (maybe it did at some point and we have a regression?), I think we should keep the mechanism as it's cheap to implement. We can just add the same PdoMap object to the PdoMaps collection under different keys. Just need to filter when getting an iterator, so the PdoMap objects are not returned multiple times.

    And we should definitely add some unit tests regarding the lookup by parameter object index. Will resume work on this when time permits.

  2. acolomb commented on Aug 5, 2025

    @acolomb
    Member

    Please refer to #609 now, which supersedes #602. Had to reopen the PR to target the master branch.

  3. added a commit that references this issue on Oct 20, 2025
    e58a653
  4. deleted a comment from iwen-sue on Mar 6, 2026
  5. added a commit that references this issue on Mar 15, 2026
    399283a
  6. dihjaywyper commented on Apr 3, 2026

    @dihjaywyper

    No   PR #613 is not fully production-ready. Although PdoBase.getitem allows access using mapping and communication parameter record IDs (such as 0x1600 and 0x1A00), PdoMaps.getitem still performs a direct dictionary lookup without translating these record IDs into internal indices. The maps dictionary is populated only with sequential keys (1 to 512), so lookups using record IDs will fail. This means the fix is incomplete and the advertised functionality is not actually implemented, making the solution incorrect and not production-ready.

  7. added a commit that references this issue on Apr 4, 2026
  8. acolomb commented on Apr 7, 2026

    @acolomb
    Member

    @dihjaywyper if you want to talk about actual issues, please look at the most recent commit and do your homework. Happy to discuss any issues you find, filed as such on GitHub. But please don't abuse this issue with outdated comments.

  9. added a commit that references this issue on Apr 17, 2026
    ac4dbf1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions