Rhoban / onshape-to-robot

Converting OnShape assembly to robot definition (SDF or URDF) through OnShape API
MIT License
237 stars 55 forks source link

Fix KeyError when adding frames #66

Closed rsinnet closed 2 years ago

rsinnet commented 2 years ago

Use defaultdict to add an empty list for the key if one does not exist.

Gregwar commented 2 years ago

Can you explain a way to reproduce a bug without this patch ?

rsinnet commented 2 years ago

Can you explain a way to reproduce a bug without this patch ?

Sure! I tried to read the instructions carefully but still I might be doing it wrong, so apologies!

I created a box and added to an assembly. Then I added a "Connector mate" to a face of the box. Then I inserted your frame part linked from the documentation. Then I created a "Fastened mate" to tie them together.

I made the document publicly accessible so this may work for you:

{
  "documentId": "8e8d1b3d5505481031c029ae",
  "assemblyName": "Assembly",
  "outputFormat": "urdf",
  "useScads": true,
  "drawCollisions": true,
  "drawFrames": true,
  "addDummyBaseLink": true,
  "robotName": "robot",
  "simplifySTLs": "collision"
}

I just tried this with and with out this patch. When I run it without, I get:

* Checking OpenSCAD presence...
* Checking MeshLab presence...

* Retrieving workspace ID ...
+ Using workspace id: a650a248857ecbb699ad09b2

* Retrieving elements in the document, searching for the assembly...
+ Found assembly, id: aecfe72e9baca1ebdb9c9179, name: "Assembly"

* Retrieving assembly "Assembly" with id aecfe72e9baca1ebdb9c9179

* Getting assembly features, scanning for DOFs...
* Found total 0 DOFs
Traceback (most recent call last):
  File "/home/vscode/.local/bin/onshape-to-robot", line 5, in <module>
    from onshape_to_robot import onshape_to_robot
  File "/home/vscode/.local/lib/python3.8/site-packages/onshape_to_robot/onshape_to_robot.py", line 13, in <module>
    from .load_robot import \
  File "/home/vscode/.local/lib/python3.8/site-packages/onshape_to_robot/load_robot.py", line 294, in <module>
    frames[occurrenceA].append(
KeyError: 'MpgpnAHi2bemYjTci'

After looking through and running in the debugger, I found that the dictionary expects keys to exist with lists as values and that the dictionary wasn't being properly initialized, so I changed the dictionary to initialize with an empty list for new keys.

Gregwar commented 2 years ago

I feel like this problem comes from the fact that the children should appear first when mating, and here it doesn't.

This constraint is maybe annoying and could be replaced with something smarted. Maybe we could specify the top-level link and let onshape to robot organize itself the tree without having to bother about children/parent matings.

EDIT: the problem is something else here, I am investigating

Gregwar commented 2 years ago

Ok the code is a little weird; frames are initialized when we meet a new link (from DOF point of view). I think your fix is good, but you might also remove the following code that is becoming useless:

if child not in frames:
    frames[child] = []
if parent not in frames:
    frames[parent] = []

In that case

rsinnet commented 2 years ago

Ok the code is a little weird; frames are initialized when we meet a new link (from DOF point of view). I think your fix is good, but you might also remove the following code that is becoming useless:

if child not in frames:
    frames[child] = []
if parent not in frames:
    frames[parent] = []

In that case

Ah good point. I removed the obsolete code per your suggestion. Thanks for catching that.

Gregwar commented 2 years ago

Thanks for contributing