Skip to content

Keep motor indices in the order passed to init_arm_motors() - #176

Open
pradeepsuryad wants to merge 2 commits into
enactic:mainfrom
pradeepsuryad:keep-motor-order
Open

pradeepsuryad wants to merge 2 commits into
enactic:mainfrom
pradeepsuryad:keep-motor-order

Conversation

@pradeepsuryad

Copy link
Copy Markdown

Fixes #162.

DMDeviceCollection::get_dm_devices() walked CANDeviceCollection's std::map, so motor index i was the i-th motor in recv-id order rather than the i-th one passed to init_arm_motors(). This follows the proposal in #162:

  • CANDeviceCollection keeps an insertion-ordered vector next to the map, exposed as get_devices_in_order(). add_device() and remove_device() keep the two in step, and re-adding a recv id replaces that device in place. The map, and get_devices(), stay for recv-id lookup.
  • get_dm_devices() walks the ordered list, so get_motors(), get_motor(i), get_link_stats(i), the *_one(i, ...) methods and the *_all(vector) methods all follow the order the motors were passed in.

With the ids passed in ascending order nothing changes.

It also stops diagnose --id 8,1 from attaching rows to the wrong ids: run_diagnose() labels its rows from the ids in command-line order and reads their stats with get_link_stats(i). A repeated id such as --id 1,1 still leaves an empty row; I've left that for #161.

Test

No motors needed. On vcan0 (GitHub Actions, Ubuntu 22.04 and 24.04), a small program runs the example from #162 (ids 0x02, 0x01 with MIT, POS_VEL) and an ascending case. For each, it checks that get_motors() is in the order the ids were passed, and that each motor accepts a command in the mode it was given (get_link_stats(i): sent, not rejected).

On main, the #162 case fails all three checks; with this branch both cases pass: https://github.com/pradeepsuryad/openarm_can/actions/runs/42076008764

The check program is on a separate branch of my fork. I can add it to the repository as a test if you'd like one.

DMDeviceCollection indexed motors by their position in get_dm_devices(),
which walked CANDeviceCollection's std::map and so came out in recv-id
order. With the ids passed out of ascending order, per-motor settings went
to the wrong motors: init_arm_motors({DM4310, DM4310}, {0x02, 0x01},
{0x12, 0x11}, {MIT, POS_VEL}) gave MIT to 0x01.

Keep an insertion-ordered vector next to the map, which stays for recv-id
lookup, and have get_dm_devices() walk it. Re-adding a recv id replaces
that device in place.

Fixes enactic#162.
@packit-as-a-service

Copy link
Copy Markdown

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo dnf install -y 'dnf*-command(copr)'
  • dnf copr enable packit/enactic-openarm_can-176
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

This branch has not been deployed

No deployments
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.

Keep motor index order equal to the order passed to init_arm_motors()

1 participant