Add basic motion profiling #36

AndrewLester · 2019-01-06T21:59:55Z

Closes #35

AndrewLester · 2019-01-06T22:00:38Z

Note: The current stats for width/height/mass of the test bot are incorrect. I will get the right ones tomorrow, so just ignore this pull for now. As well as this, we can't actually use encoders on the test bot currently.

ErikBoesen · 2019-01-06T22:02:05Z

robot/physics.py

+
+
+class PhysicsEngine(object):
+    '''


I'm tempted to complain about your comment formatting here but this PR is so fantastic that I have a hard time doing that

Thank you haha. I'll fix those quotes and check with pycodestyle as well.

ErikBoesen · 2019-01-06T22:03:30Z

robot/trajectory_generator.py

+TRAJECTORY_DIRECTORY = 'trajectories'
+
+
+trajectories = {


Is there a better way to do this then listing them in trajectory_generator? This feels like something that would fit better in autonomous files. @ArchdukeTim may have better intuition on this.

You could read them in from a JSON? Pathfinder will also read in directly from a pathfinder binary created by he serialize methods, and you can also do a CSV. I think the best alternative would be to store them all in a JSON file, but I don’t think it’s necessarily bad to do it here

ArchdukeTim · 2019-01-06T22:30:52Z

robot/components/trajectory_follower.py

+        self.right_follower.configureEncoder(self.r_encoder.get(), 360, self.WHEEL_DIAMETER)
+
+    def is_following(self, trajectory_name):
+        if self._current_trajectory == trajectory_name:


You should change this to if self._current_trajectory is not None so that passing in None Before you’ve given it a trajectory to follow doesn’t return True. Edge case, but good practice

ArchdukeTim · 2019-01-06T22:31:33Z

robot/components/trajectory_follower.py

+    def is_following(self, trajectory_name):
+        if self._current_trajectory == trajectory_name:
+            return True
+        return False


Personal preference, but wheneve you’re using an if condition to return true or false, just do return theCondition()

This is just good practice in general.

ArchdukeTim · 2019-01-06T22:32:30Z

robot/physics.py

+            6*units.inch                        # wheel diameter
+        )
+
+        self.kEncoder = 360 / (0.5 * math.pi)


This value seems mighty mighty low.

Those are some SMALL wheels.

I think they're actually 6 inches, we can check to be sure @AndrewLester

Maybe a good idea to make this a constant at any rate.

It's in feet. I think I just kept it that way for consistency, but I'm not sure. The 360 is low though, as I think it's the encoder counts per revolution which is normally higher.

that makes more sense, but then the encoder value still seems low I think

ArchdukeTim · 2019-01-06T22:34:52Z

robot/trajectory_generator.py

+TRAJECTORY_DIRECTORY = 'trajectories'
+
+
+trajectories = {


You could read them in from a JSON? Pathfinder will also read in directly from a pathfinder binary created by he serialize methods, and you can also do a CSV. I think the best alternative would be to store them all in a JSON file, but I don’t think it’s necessarily bad to do it here

ArchdukeTim · 2019-01-06T22:35:41Z

robot/trajectory_generator.py

+        generated_trajectories = _generate_trajectories()
+        _write_trajectories(generated_trajectories)
+    else:
+        with open(os.path.join(os.path.dirname(__file__) + os.sep + TRAJECTORY_DIRECTORY, 'trajectories.pickle'), 'rb') as f:


look at the docs for join. There’s a better way to do this

ErikBoesen · 2019-01-06T22:33:28Z

requirements.txt

@@ -1,3 +1,5 @@
 pyfrc>=2018.0.0
 robotpy-ctre>=2018.0.0
 pynetworktables>=2018.0.0
+pybind11>=2.2.2


Our favorite!

Did this give you trouble in the past?

I believe we forgot to install it on the RIO last year and it took us a while to figure out what we'd done wrong.

You shouldn't need to install this on the RIO. If you do, you're doing something wrong.

@virtuald I'm likely thinking of a different package then.

ErikBoesen · 2019-01-06T22:37:44Z

robot/components/trajectory_follower.py

+    Move along generated paths for autonomous
+    """
+    # TODO FIND THE REAL VALUES
+    WHEEL_DIAMETER = 0.5


Fairly sure the diameter is six inches but we can check

ErikBoesen · 2019-01-06T22:38:20Z

robot/physics.py

@@ -0,0 +1,69 @@
+#
+# See the notes for the other physics sample


Don't think you want this

ErikBoesen · 2019-01-06T22:38:43Z

robot/physics.py

+
+    def update_sim(self, hal_data, now, tm_diff):
+        '''
+            Called when the simulation parameters for the program need to be


Yeah this is indented a little differently from how we normally do

ErikBoesen · 2019-01-06T22:38:54Z

robot/robot.py


    def createObjects(self):
        # Joysticks
+


But why tho

ErikBoesen · 2019-01-06T22:39:11Z

robot/sim/config.json

@@ -0,0 +1,18 @@
+{
+  "pyfrc": {
+


You can probably get rid of this spacing dunno

ErikBoesen · 2019-01-06T22:41:56Z

If Tim is comfortable with this then I am too. I admittedly don't understand MP well enough to make actual substantive criticism on your methods.

ArchdukeTim · 2019-01-07T01:04:07Z

robot/robot.py

 import wpilib.drive
+from robotpy_ext.common_drivers import navx


Update your robotpy-wpilib-utilties

ArchdukeTim · 2019-01-07T04:50:47Z

robot/autonomous/charge.py

-from components import drive
+# from automations import
+# from magicbot import tunable
+from components import drive, trajectory_follower


 class Charge(AutonomousStateMachine):
    MODE_NAME = 'Charge'
    DEFAULT = True

    drive = drive.Drive


you should change this to be drive: drive.Drive for consistency

ArchdukeTim · 2019-01-07T04:51:34Z

robot/components/trajectory_follower.py

+    navx: navx.AHRS
+    l_encoder: wpilib.Encoder
+    r_encoder: wpilib.Encoder
+    generated_trajectories: Dict


dict not Dict. While this is a type hint, we're abusing it for our purposes, and the class that it is is dict

That's actually really cool. So magic bot looks at the type hint and uses it to match the object?

mhmm. Technically you could also do injectable = injectableType but we thought it made more sense to use annotations

ArchdukeTim · 2019-01-07T04:52:10Z

robot/components/trajectory_follower.py

+        self.right_follower.configureEncoder(self.r_encoder.get(), 360, self.WHEEL_DIAMETER)
+
+    def is_following(self, trajectory_name):
+        return (self._current_trajectory is not None and


nitpick: necessary parens

What exactly do you mean by necessary parens? Do I not need the one surrounding both statements?

Yeah you don’t need them. I meant to say unnecessary parens

ArchdukeTim · 2019-01-07T04:54:55Z

robot/components/trajectory_follower.py

+
+    def execute(self):
+        if (self.left_follower.trajectory is None or self.right_follower.trajectory is None) or \
+           (self.left_follower.isFinished() and self.right_follower.isFinished()):


nitpick: also technically necessary parens

Lol I just saw he went the wrong way. Darn you autocorrect.

ArchdukeTim · 2019-01-07T04:55:43Z

robot/physics.py

+
+        self.drivetrain = tankmodel.TankModel.theory(
+            motor_cfgs.MOTOR_CFG_CIM,           # motor configuration
+            110 * units.lbs,                      # robot mass


Bump this up to 140-150 too

The kit robot only weighs 80ish pounds. Why 140-150?

Oh that’s right this is not the real robot. Ok just change it when you have a measurement

ArchdukeTim · 2019-01-07T05:04:55Z

robot/robot.py

@@ -76,4 +91,4 @@ def teleopPeriodic(self):


 if __name__ == '__main__':
-    wpilib.run(Bot)
+    wpilib.run(Bot, physics_enabled=True)


It took me a while to figure out why you had this, but then I checked the examples. Funnily enough, this doesn't actually do anything. Even if you set it to false 😂

ErikBoesen · 2019-01-07T14:07:05Z

robot/physics.py

+
+        self.physics_controller = physics_controller
+
+        # Change these parameters to fit your robot!


Cut out all the comments that are only pertinent to example code.

ErikBoesen · 2019-01-07T14:07:34Z

robot/physics.py

+
+        self.drivetrain = tankmodel.TankModel.theory(
+            motor_cfgs.MOTOR_CFG_CIM,           # motor configuration
+            110 * units.lbs,                      # robot mass


I know I'm really nitpicking hard here (and this is probably from the example you used) but pounds are a unit of weight, not mass.

ErikBoesen reviewed Jan 6, 2019

View reviewed changes

AndrewLester requested review from ErikBoesen, ArchdukeTim and JosephCarpenter January 6, 2019 22:31

ArchdukeTim suggested changes Jan 6, 2019

View reviewed changes

ErikBoesen approved these changes Jan 6, 2019

View reviewed changes

AndrewLester force-pushed the master branch 2 times, most recently from fef746f to b1bef71 Compare January 6, 2019 23:23

ArchdukeTim suggested changes Jan 7, 2019

View reviewed changes

robot/robot.py Outdated

import wpilib.drive

from robotpy_ext.common_drivers import navx

Copy link

Member

ArchdukeTim Jan 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update your robotpy-wpilib-utilties

ArchdukeTim suggested changes Jan 7, 2019

View reviewed changes

ErikBoesen suggested changes Jan 7, 2019

View reviewed changes

AndrewLester force-pushed the master branch 2 times, most recently from 0eb6cd0 to 975a463 Compare January 8, 2019 14:20

AndrewLester added 8 commits January 21, 2019 00:31

Add sim framework

8eb55da

Add trajectory generator and improve follower

67fb639

Add working motion profiling

fcbd728

Rewrite trajectory loading and generating

0e96fd7

Add function to stop autonomous state after path ends

66c7186

Fix styling issues in new files

d33329b

Change to correct kitbot measurements

e63ac19

Change robot mass in sim config

563a035

AndrewLester force-pushed the master branch from 4fd35b1 to 563a035 Compare January 21, 2019 05:33

		@@ -0,0 +1,69 @@
		#
		# See the notes for the other physics sample

		import wpilib.drive
		from robotpy_ext.common_drivers import navx


		self.physics_controller = physics_controller

		# Change these parameters to fit your robot!

		TRAJECTORY_DIRECTORY = 'trajectories'


		trajectories = {

		TRAJECTORY_DIRECTORY = 'trajectories'


		trajectories = {

Add basic motion profiling #36

Are you sure you want to change the base?

Add basic motion profiling #36

Conversation

AndrewLester commented Jan 6, 2019

AndrewLester commented Jan 6, 2019 • edited Loading

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

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

ErikBoesen commented Jan 6, 2019

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

AndrewLester Jan 7, 2019 • edited Loading

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

AndrewLester commented Jan 6, 2019 •

edited

Loading

AndrewLester Jan 7, 2019 •

edited

Loading