API review

Proposer: Jack O'Quin

Present at review:

  • Joerg Baier
  • Osvaldo Castellanos
  • Zachary Sais
  • Michael Quinlan
  • Joshua Bennett
  • Enrique Sada
  • Collin Hazlett

The main focus of this review is the new art_msgs/CarControl2 message. We need to define the meanings of all combinations of the parameters.

For convenient reference, here is a copy of the message source file: 

# car acceleration control command
# $Id: CarControl2.msg 1161 2011-03-26 02:10:49Z jack.oquin $

float32 acceleration            # (m/s^2), negative is deceleration
float32 goal_velocity           # (m/s), negative is reverse
float32 steering_angle          # steering angle (radians)

# gear numbers
uint8 Drive = 0
uint8 Park = 1
uint8 Reverse = 2

uint8 gear                      # requested gear

Question / concerns / comments

Enter your thoughts on the API and any questions / concerns you have here. Please sign your name. Anything you want to address in the API review should be marked down here before the start of the meeting.

  • (Jack) it would reduce confusion to unify the gear numbers in this message with the ones used by the art_msgs/Shifter message]. The best idea I've had so far is to define another GearNumber message containing only the constants, and use those numbers for both messages.

    • (Joerg, Zach, Oz) Or change the gear numbers to match the ones in Shifter message, but a GearNumber message would definitely be more practical

      • (Jack) The API might be slightly cleaner if both messages referred to a common set of constants defined in a third message. But, as a practical implementation matter, your suggestion is easier (less code to update), and still seems like a reasonable alternative.

      • (Jack) Looking at the code, it should be relatively safe and easy to remove the gear numbers from CarControl2 and use the Shifter gear numberings instead. A simple comment should make this clear, and will be much less confusing than using two different numberings. So, I am leaning strongly towards that solution.

        • We do need to define what the Reset (0) gear number means. My initial idea is that a current gear of Reset means no message has yet been received from the shifter node. A target command with gear number Reset matches the current gear.

  • (Jack) I recommend defining zero acceleration to request direct control to the goal_velocity.

  • (Jack) At some convenient time (probably end of semester) I'd like to rename goal_velocity simply velocity.

    • (Jack) On second thought, in physics velocity is a vector and "speed" is its scalar absolute magnitude. If we define this field to be non-negative, it should be called speed.

  • (Jack) I think we should consider deprecating the "negative velocity implies reverse" rule. Pilot can still accept that as input, but issue a warning message and ignore the sign of the velocity.

  • (Jack) The steering_angle could use a more precise description, something like "average yaw of the front wheels in the vehicle frame of reference (left is positive)." Clarify that this assumes Ackermann steering, but averages out the slightly different angles of the front wheels.

    • (Michael) At a minimum the signs (+ for left, - for right) need to be described.

    • (Enrique) When we say "steering" is it "car-wheel-angle"? "Steering" might be ambiguous...

  • (Jack) We should consider renaming the message if anyone has a better idea. Since we are making incompatible changes to the API, all code currently using it will need to be modified at least slightly. CarControl2 came from the fact that it is a replacement for CarControl, but the two messages need to coexist for a while. There is nothing much to like about that name.

    • (Michael) I'm not a big fan of the name. Car Control is more the name of the entire operation rather then implying message. I'd prefer something along the lines of PilotCommand.

    • (Michael) On a similar note I find the existence of a art_msgs/CarCommand to be confusing. I'm no longer 100% sure why that message type exists as it's just a Header and CarControl packaged together.

    • (Jack) Agreed. I'm currently thinking of calling this message CarDrive and the one with a header CarDriveStamped. That follows common ROS usage.

    • (Josh) I'm in favor of CarDrive.

  • (Jack) It would be very useful to have some way for a teleop controller to tell the pilot to put the transmission in Park and then release the brake. (I would not want Park always to do that.) There is currently no way to make such a request using this interface. We could invent another gear number to accomplish it, or perhaps assign that meaning to a Reset gear command.

    • (Jack) On further reflection, using fake, overloaded gear numbers is a really bad solution. It would be better to add a new field for pilot behavior requests. The LearningCommand preemption message could be handled as a behavior. So, could releasing the brake. We could compatibly add new behaviors, as needed.

  • (Jack) Since acceleration is just a limit on the rate of speed change, should we add jerk (3rd derivative)? What about snap, crackle and pop (4th, 5th and 6th derivatives)?--

    • (Michael) I think having jerk and possibly snap in the message is a good idea. The actual pilot implementation can ignore the values until we have a proper use for them

  • (Michael) Should there be a minimum acceleration in the message. For out implementation it's not a big deal, but it is plausible to think of a controller that may take an acceleration range

    • (Jack) Maybe. I'm starting to think of acceleration as a (positive) upper limit, with -acceleration as the corresponding lower limit.

    • (Jack) Should there be ranges for jerk and snap, too? I'm thinking the sign of these limits is not very important. Whether positive or negative they represent forces applied and their derivatives.

Preliminary proposal for the final message text: 

# Driving command for a car-like vehicle using Ackermann steering.
# $Id: CarDrive.msg -1   $

# Drive at requested speed, limited by acceleration. Speed is a
# non-negative scalar, direction determined by the gear. Zero
# acceleration means as quickly as possible.
float32 speed                   # magnitude of velocity vector (m/s)
float32 acceleration            # acceleration limit (m/s^2)

# Assumes Ackermann steering. This angle is the average yaw of the
# front wheels in the vehicle frame of reference (positive left),
# modeling their slightly differing wheel angles as a tricycle.
float32 steering_angle          # steering angle (radians)

# Gear numbers are defined in the art_msgs/Shifter message. 
# "Reset" means to use the current gear.
uint8 gear                      # requested gear

# Requested behavior.
uint8 behavior
uint8 Run = 0                   # normal driving
uint8 Pause = 1                 # stop issuing servo commands
uint8 Off = 2                   # turn off (some) devices

Meeting agenda

After discussing the API on this wiki page, I'd like to close this review after class next Wednesday, 2011-04-20.

The class discussion was helpful, issues raised there are listed in the conclusions below.

Conclusion

Action items that need to be taken:

  • /!\ Define zero acceleration to mean "no limit".

  • /!\ Rename goal_velocity to speed, which should always be positive, even when travelling in Reverse.

  • /!\ Rename the messages CarDrive and CarDriveStamped.

  • /!\ Add jerk as well as acceleration. Some pilot controllers will ignore it, but it makes sense for requesters.

  • /!\ Combine gear numbering of this message and Shifter into a common sub-message, making it clear they are identical.

  • /!\ Clarify that steering_angle describes the angle of the front wheels, not the position of the steering wheel in the driver's compartment.

  • /!\ Define acceleration as a desired (absolute) value, preferably not to be exceeded. Define jerk similarly.

  • /!\ Clarify the meaning of the new PilotBehavior field.

To summarize, here are the final, agreed-upon message definitions:

CarDrive: 

# Driving command for a car-like vehicle using Ackermann steering.
# $Id: CarDrive.msg 1412 2011-04-30 15:40:53Z jack.oquin $

# Drive at requested speed, acceleration and jerk (the 1st, 2nd and
# 3rd derivatives of position). All are non-negative scalars. 
#
# Speed is defined as the scalar magnitude of the velocity
# vector. Direction (forwards or backwards) is determined by the gear.
#
# Zero acceleration means change speed as quickly as
# possible. Positive acceleration may include deceleration as needed
# to match the desired speed. It represents a desired rate and also a
# limit not to exceed.
#
# Zero jerk means change acceleration as quickly as possible. Positive
# jerk describes the desired rate of acceleration change in both
# directions (positive and negative).
#
float32 speed                   # magnitude of velocity vector (m/s)
float32 acceleration            # desired acceleration (m/s^2)
float32 jerk                    # desired jerk (m/s^3)

# Assumes Ackermann (front-wheel) steering. This angle is the average
# yaw of the two front wheels in the vehicle frame of reference
# (positive left), ignoring their slightly differing angles as if it
# were a tricycle. This is *not* the angle of the steering wheel
# inside the passenger compartment.
#
float32 steering_angle          # steering angle (radians)

Gear gear                       # requested gear (no change if Naught)
PilotBehavior behavior          # requested pilot behavior

Gear: 

# ART vehicle transmission gear numbers
#
# Used by several different messages.

# $Id: Gear.msg 1375 2011-04-25 20:59:15Z jack.oquin $

# Gear numbers. 
#
# Naught means: reset all Shifter relays; no change of CarDrive gear.
uint8 Naught = 0
uint8 Park = 1
uint8 Reverse = 2
uint8 Neutral = 3
uint8 Drive = 4
uint8 N_gears = 5

uint8 value             # requested or reported gear number

PilotBehavior: 

# ART autonomous vehicle pilot node behaviors.
#
# Normally, the pilot node does Run, continually sending commands to
# the servo device actuators and monitoring their state.  With Pause,
# the pilot becomes passive, allowing a learning algorithm or human
# controller direct access to those devices.  In the Off state,
# various devices are shut down: the transmission in Park, the brake
# released, the throttle at idle.  The engine is not turned off, but
# it could be.

# $Id: PilotBehavior.msg 1434 2011-05-03 15:11:56Z jack.oquin $

# Behavior value
uint8 value

# Behavior numbers:
uint8 Run = 0                   # normal driving
uint8 Pause = 1                 # stop issuing servo commands
uint8 Off = 2                   # turn off devices
uint8 N_behaviors = 3

CarDriveStamped: 

# CarDrive message with timestamp.
# $Id: CarDriveStamped.msg 1335 2011-04-22 02:32:41Z jack.oquin $

Header    header
CarDrive  control

Wiki: art_pilot/Reviews/2011-04-13_API_Review (last edited 2012-01-31 15:03:32 by JackOQuin)

Except where otherwise noted, the ROS wiki is licensed under the
Creative Commons Attribution 3.0