API review

Proposer: Stuart Glaser

Present at review:

• Stu
• Wim
• Melonee
• Gunter
• Eitan
• Kevin
• Brian
• Tully

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.

Tully

• All 2 letter variables in API are too short.
  • The one letter P I D are ok, but i1, i2, de etc are not clear enough.

Gunter

• Should I mention the word documentation?

• dither/ramp doc refers to sweep - I assume cut/paste from sinesweep...
• time - passed as double (sine). Should time consistently be ros::Time?

• dither isn't white noise. I would call this something else, maybe white noise

• In PID there is a divide by i_gain_ without checking!

• In SineSweep there is a divide by frequency (and others?) without checking!

Eitan

• I guess this is more of a documentation issue, but the ROS API should go on the wiki
• Maybe change init to initNode in Pid just to make things consistent
• Doc again, but explaining each module on the wiki would be valuable

Meeting agenda

To be filled out by proposer based on comments gathered during API review period

Change small names:

• getCurrentPIDErrors (double *pe, double *ie, double *de) to p_error...
• initPid (double P, double I, double D, double I1, double I2) to i_min, i_max
• And constructor for PID
• Sinusoid: change time to a ros::Time
• sin should be relative to a start time.
• If Sinusoid isn't being used, remove it.
• dither/ramp doc refers to sweep - I assume cut/paste from sinesweep...
  • Ticket these (Kevin + Melonee)
• Change Dither to Noise (Kevin)

• Ticket for checking for divide by zeros: (Stu) SineSweep, PID

• Review the 'init' functions, change all to init (NodeHandle)

  • Make sure it's not a const NodeHandle &. Just NodeHandle by value

• Ticket to change these to searchParam (Stu)
• Pid: (actual - desi or desi - actual)??
  • Whatever it becomes, clearly document it
  • Leave it as is.
  • updatePid(pos_actual - pos_desi, vel_actual - vel_desi, dt)

Conclusion

Package status change mark change manifest)

• Action items that need to be taken.

• Major issues that need to be resolved