Calzim/arm api python #40

CalZim · 2020-03-12T16:10:45Z

ported arm api for python and fixed examples to work with it

python arm api and created new versions of all the arm kit examples to use the arm api

added comments and cleaned up code

have yet to remove old dead code

also cleaned out some dead code and added some comments

rLinks234 · 2020-03-15T16:50:03Z

FYI: I changed the base branch into which this will be merged, just so we can see the diff between this branch and #39 .

rLinks234 · 2020-03-20T16:42:06Z

kits/arm/arm.py

+
+
+
+class Arm():


Inherit from object like above to be consistent. However, I think this is done automatically for Python 3, and since we don't support Python 2 anymore, we can probably just get rid of this explicit inheritance...

rLinks234 · 2020-03-20T16:45:22Z

kits/arm/arm.py

+    def __init__(self, params):
+        # Create arm from lookup
+        self.lookup = hebi.Lookup()
+        sleep(2)


I really don't want to do this in the constructor... We should probably expose the Arm API as something like the following:

def create_arm(params, lookup_sleep_amount=2, other_input_parameters...): lookup = hebi.Lookup() sleep(lookup_sleep_amount) group = lookup.get_group_from_names([params.family], params.module_names) if group is None: raise RuntimeError("Could not create group") # Probably want to raise exception here on failure IMO? return Arm(group) # Can add other constructor parameters as needed here too

I ran into something similar in MATLAB and I'm a big fan of just passing in the group directly. The lookup has so many settings and overloads that it seems easier to just use the existing API rather than replicating it via factory methods.

rLinks234 · 2020-03-20T16:46:28Z

kits/arm/arm.py

+
+        # Create robot model
+        try:
+            self.model = hebi.robot_model.import_from_hrdf(params.hrdf)


Same as above. This should probably be moved outside of the constructor and into the create_arm comment I posted above. An Arm shouldn't be ever created with an invalid hrdf file (i.e., we want to prevent "incomplete/invalid" state like this)

rLinks234 · 2020-03-20T16:46:45Z

kits/arm/arm.py

+            self.model = hebi.robot_model.import_from_hrdf(params.hrdf)
+        except Exception as e:
+            print('Could not load HRDF: {0}'.format(e))
+            sys.exit()


definitely should remove this exit() call...

rLinks234 · 2020-03-20T16:47:34Z

kits/arm/arm.py

+
+        self.set_goal_first = True
+        self.at_goal = True
+        self.goal = "grav comp"


Generally, we have instance variables starting with an underscore (or two, to make them "private"/mangled) and create getters/setters as needed to modify them. I'd prefer that for this class as well.

rLinks234 · 2020-03-20T16:48:52Z

kits/arm/arm.py

+    def loadGains(self, gains_file):
+        try:
+            self.cmd.read_gains(gains_file)
+            if not self.grp.send_command_with_acknowledgement(self.cmd):


We should definitely try sending gains multiple times. This warrants a separate discussion, but it's relevant enough to comment on for now.

MATLAB just added a generic sendWithRetry that works for gains and other commands. Maybe that would be useful here as well: https://github.com/HebiRobotics/hebi-matlab-examples/blob/master/include/hebi/HebiUtils.m#L822-L855

rLinks234 · 2020-03-20T16:49:16Z

kits/arm/arm.py

+
+    # Updates feedback and generates the basic command for this timestep
+    def update(self):
+        self.prev_fbk = list(self.fbk)


Why is this a list?

I'm responsible for this. This clones the IOState. Before this was

self.prev_fbk = self.fbk

I guess the less cryptic way to do this would be

self.prev_list = self.fbk.copy()

This wasn't a thing until Python3.3, so I remembered the list(list) way instead

Is feedback.copy_from the behavior you want here?

rLinks234 · 2020-03-20T16:58:30Z

kits/arm/arm.py

+        return
+
+
+    def createMotionArray(self, size, prev_cmd, array=None, flow=None):


Should probably use snek_case and not camelCase in this class too, btw...

rLinks234 · 2020-03-20T17:01:06Z

kits/arm/arm.py

+        # Note flow overrides any array passed (used for vel and accel, should never pass flow and positions)
+        mArray = np.empty((self.grp.size, size+1))
+        mArray[:, 0] = prev_cmd
+        if flow == None:


Pedantic thing: Apparently it's better practice to do if flow is None: here instead.

I think the rationale is that None is a "singleton" of sorts... There is only one None instance in a Python interpreter. That means that if flow is equal to None (literally, if flow is "null"), it's underlying object is the same object id (i.e., both variables point to the same object in memory) as None, meaning you can use the is operator.

rLinks234 · 2020-03-20T17:02:48Z

kits/arm/arm.py

+
+
+    def createGoal(self, position, durration=None, velocity=None, accel=None, flow=None):
+        # Create trajectory to target position


Thanks for writing up documentation on the parameters here. One thing though:

We should do this whole comment as a multi line comment (like the Ex: ... below it), meaning this should be replaced with:

def create_goal(params....): """ Create trajectory to target position ... Blah Blah ... ...... Ex: 3dof arm with a 2 point trajectory .....

rLinks234 · 2020-03-20T17:05:22Z

kits/arm/ex_AR_kit.py

@@ -30,7 +30,7 @@

 # Mobile device setup
 phone_family = 'HEBI'
-phone_name = "Cal's iPhone"
+phone_name = "Mobile IO"


I think the naming convention is mobileIO now. Could be wrong though... I always forget myself.

Yes, it's mobileIO

ennerf · 2020-03-23T16:54:37Z

kits/arm/arm.py

+    def __init__(self, family, moduleNames, hrdf):
+        self.family = family
+        self.hrdf = hrdf
+        self.moduleNames = moduleNames


different casing than other names?

ennerf · 2020-03-23T16:59:31Z

kits/arm/arm.py

+
+
+    # Loads gains from given gains file    
+    def loadGains(self, gains_file):


I think that this function should accept a gains object rather than a filename. There are plenty of cases where we would want to load some gains and modify them before setting (e.g. the impedance control example increases the effort kp by 2x).

That being said, this would be a good candidate for a helper function rather than part of the Arm API.

ennerf · 2020-03-23T18:36:51Z

kits/arm/ex_teach_repeat.py

@@ -193,7 +193,7 @@ def run():
  print_and_cr("When in playback mode, 'b6' resumes training, and 'b1' quits.")


-  m = mbio.MobileIO("HEBI", "Phone Name")
+  m = mbio.MobileIO("HEBI", "Mobile IO")


alexhongs added 13 commits March 10, 2020 11:56

arm api and updated arm kit examples

29a6dac

python arm api and created new versions of all the arm kit examples to use the arm api

added comments

7b8c99a

added comments and cleaned up code

implemented multiwaypoint trajectory planning with flow

db49a4a

fixed examples to work with updated arm api

9c52639

added helper functions

f34e4c2

have yet to remove old dead code

added create and set goal as seperate functions

d19c21b

also cleaned out some dead code and added some comments

fixed examples to work with updated arm api

109e225

fixed comments and prints

11e9cd5

more comments

e91cade

comments

7008324

removed old versions

e503833

removed more old versions

9f7f211

naming conventions

04ce4b0

CalZim assigned rLinks234 and cwbollinger Mar 12, 2020

comments and missed changes

cf85e2c

rLinks234 self-requested a review March 15, 2020 16:11

rLinks234 changed the base branch from master to calzim/example-cleanup March 15, 2020 16:49

iamtesch changed the base branch from calzim/example-cleanup to calzim/only_example_cleanup March 20, 2020 14:20

rLinks234 reviewed Mar 20, 2020

View reviewed changes

ennerf reviewed Mar 23, 2020

View reviewed changes

cwbollinger closed this Sep 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Calzim/arm api python #40

Calzim/arm api python #40

CalZim commented Mar 12, 2020

rLinks234 commented Mar 15, 2020

rLinks234 Mar 20, 2020

rLinks234 Mar 20, 2020

ennerf Mar 23, 2020

rLinks234 Mar 20, 2020

rLinks234 Mar 20, 2020

rLinks234 Mar 20, 2020

rLinks234 Mar 20, 2020

ennerf Mar 23, 2020

rLinks234 Mar 20, 2020

cwbollinger Mar 23, 2020

rLinks234 Mar 23, 2020

rLinks234 Mar 20, 2020

rLinks234 Mar 20, 2020

rLinks234 Mar 20, 2020

rLinks234 Mar 20, 2020

ennerf Mar 23, 2020

ennerf Mar 23, 2020

ennerf Mar 23, 2020

ennerf Mar 23, 2020

		return


		def createMotionArray(self, size, prev_cmd, array=None, flow=None):



		def createGoal(self, position, durration=None, velocity=None, accel=None, flow=None):
		# Create trajectory to target position



		# Loads gains from given gains file
		def loadGains(self, gains_file):

Calzim/arm api python #40

Calzim/arm api python #40

Conversation

CalZim commented Mar 12, 2020

rLinks234 commented Mar 15, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment