API review
Proposer: Melonee Wise
Present at review:
- Chad Rockey
- Ken Tossell
- Dave Hershberger
- Tully Foote
Overview
As part of electric, I am adding the capability to set the leds and rumble level of the ps3 controller. For this I am proposing two new msgs for setting the rumble and led. I have implemented the proposed changes in trunk for people to try out.
Follow the these tutorials to pair the ps3joy stick and run the driver: http://www.ros.org/wiki/ps3joy/Tutorials/PairingJoystickAndBluetoothDongle
Then you can set the leds by calling:
rostopic pub /ps3joy/set_led ps3joy/SetLED 1 0 0 1
or set the rumble by calling:
rostopic pub /ps3joy/set_led ps3joy/SetRumble 255 0
Proposed Messages
# Msg for turning the LEDs on the PS3 controller on and off #define on and off uint8 ON = 1 uint8 OFF = 0 uint8 led1 uint8 led2 uint8 led3 uint8 led4
# The PS3 has a low and high frequency rumbler you can set the intensity from 0 - 255. # To give an on/off feeling set the value to 255 and then to 0 # The low frequency rumble feels "stronger". uint8 rumble_low # you can feel values from ~230-255 uint8 rumble_high # you can feel values from ~20-255
Possible Generalized Messages
joy/Feedback
# Issues Durational LED and Vibration Feedback to user. float32[] led_time # time for led to remain lit in seconds ( 0.0 == don't turn on, INF == stay on until next command ) float32[] rumble_intensity # the intensity for each rumble motor ( 0.0 == off, 1.0 == max ) float32[] rumble_time # time for rumble to remain on in seconds ( 0.0 == don't turn on, INF == stay on until next command )
Another Possibility
joy/FeedbackArray
Feedback[] array
joy/Feedback
string LED1="led1" string LED2="led2" string LED3="led3" string LED4="led4" string RUMBLE_LOW="rumble" # For single-rumbler devices, all these refer to the same thing. string RUMBLE_LOW="rumble_low" string RUMBLE_HIGH="rumble_high" string name FeedbackSetting[] sequence bool loop # true means repeat the sequence forever float32 final_intensity # ignored if loop == true
joy/FeedbackSetting
# Intensity of the feedback, from 0.0 to 1.0, inclusive. If device is # actually binary, driver should treat 0<=x<0.5 as off, 0.5<=x<=1 as on. float32 intensity # Time in seconds for this intensity to remain in force. float32 duration
Example uses
These examples take advantage of the fact that the boolean "loop" defaults to false.
To turn on LED 1
Feedback f; f.name = "led1"; f.final_intensity = 1.0; FeedbackArray fa; fa.array.push_back(f); pub.publish(fa);
To do a repeating on-off rumble sequence with 70% duty cycle
Feedback f; f.name = "rumble_high"; f.loop = true; f.sequence.setSize(2); f.sequence[0].intensity = 1.0; f.sequence[0].duration = 0.7; f.sequence[1].intensity = 0.0; f.sequence[1].duration = 0.3; FeedbackArray fa; fa.array.push_back(f); pub.publish(fa);
To rumble for 1 second and stop
Feedback f; f.name = "rumble_high"; f.sequence.setSize(1); f.sequence[0].intensity = 1.0; f.sequence[0].duration = 1.0; f.final_intensity = 0.0; FeedbackArray fa; fa.array.push_back(f); pub.publish(fa);
To turn LEDs 1,2,3,4 to off,on,on,off respectively
FeedbackArray fa; fa.array.resize(4); fa.array[0].name = LED1 fa.array[1].name = LED2 fa.array[1].final_intensity = 1; fa.array[2].name = LED3 fa.array[2].final_intensity = 1; fa.array[3].name = LED4 pub.publish(fa);
Another Possibility Refined
joy/FeedbackSequenceArray
# This message holds sequences of feedback for publishing values for multiple actuators at once. Intended for parallel publishing of sequences. # This is a very complicated message and could easily produce conflicts. Is it wanted? FeedbackSequence[] array
joy/FeedbackArray
# This message publishes values for multiple actuators at once. Intended for parallel execution. Feedback[] array
joy/FeedbackSequence
# This message allows pulsing and timing of feedback. Intended for serial execution. # Note: you can sequence different feedback types within one sequence. Feedback[] sequence bool loop # true means repeat the sequence forever
joy/Feedback
# Declare of the type of feedback uint8 TYPE_LED = 0 uint8 TYPE_RUMBLE = 1 uint8 TYPE_BUZZER = 2 uint8 type # This will hold an id number for each type of each feedback. # Example, the first led would be id=0, the second would be id=1 uint8 id # Intensity of the feedback, from 0.0 to 1.0, inclusive. If device is # actually binary, driver should treat 0<=x<0.5 as off, 0.5<=x<=1 as on. float32 intensity # Time in seconds for this intensity to remain in force. A value of INF will mean to stay at this value until commanded otherwise. float32 duration
Example uses
To turn on LED 0
Feedback f; f.type = TYPE_LED; f.id = 0; f.intensity = 1.0; f.duration = INF; pub.publish(f);
To do a repeating on-off rumble sequence with 70% duty cycle
Feedback f; f.type = TYPE_RUMBLE; f.id = 0; f.intensity = 1.0; f.duration = 0.7; Feedback f2; f2.type = TYPE_RUMBLE; f2.id = 0; f2.intensity = 0.0; f2.duration = 0.3; FeedbackSequence fs; fs.sequence.push_back(f1); fs.sequence.push_back(f2); fs.loop = true; pub.publish(fs);
To rumble for 1 second and stop
Feedback f; f.type = TYPE_RUMBLE; f.id = 0; f.intensity = 1.0; f.duration = 1.0; pub.publish(f);
To turn LEDs 1,2,3,4 to off,on,on,off respectively
Feedback f; f.type = TYPE_LED; f.id = 0; f.intensity = 0; f.duration = INF; Feedback f2; f2.type = TYPE_LED; f2.id = 1; f2.intensity = 1; f2.duration = INF: Feedback f3; f3.type = TYPE_LED; f3.id = 2; f3.intensity = 1; f3.duration = INF; Feedback f4; f4.type = TYPE_LED; f4.id = 3; f4.intensity = 0; f4.duration = INF; FeedbackArray fa; fa.array.push_back(f); fa.array.push_back(f1); fa.array.push_back(f2); fa.array.push_back(f3); pub.publish(fa);
To blink and then vibrate in sequence.
Feedback fled; fled.type = TYPE_LED; fled.id = 0; fled.intensity = 1.0; fled.duration = 0.5; Feedback frum; frum.type = TYPE_RUMBLE; frum.id = 0; frum.intensity = 0.7; frum.duration = 0.5; FeedbackSequence fs; fs.sequence.push_back(fled); fs.sequence.push_back(frum); fs.loop = true; pub.publish(fs);
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.
(Chad) Do we want to make this a more generic part of joystick drivers and include variable length led[] and vibration[] fields? This would be consistent with the axes and buttons of the Joy message.
(Melonee) do you know of another joystick with rumble feedback and leds? it would be nice to have a sense of what other types of interfaces these devices have.
(Chad) The Wiimote has its own unique ROS messages currently. There are also some actual joysticks that include rumble support as well. Would there be an advantage to merging the message types and just saying that the PS3 driver listens to LEDs 0 through 3?
(Melonee) yes but I'm not sure that you can change the actual intensity of the rumble in the wiimote only toggle it on and off which is different from the ps3 in which you can change the intensity
(Chad) I've confirmed on http://wiibrew.org/wiki/Wiimote that Wii rumble is simple on or off. I think a driver for any device would interpret as need be. The Wii could simple have 0.0 be off and anything greater be on.
(Chad) I think if it were more generic, rumble intensity could just be a double from 0.0 to 1.0.
(Ken) Support for flashing LEDs would be nice: period[4], duty_cycle[4]. Or period[n] != 0 could mean that the ledN+1 variable is the duty cycle.
- I just found some information on the flash control part of the message:
* the total time the led is active (0xff means forever) * | duty_length: how long a cycle is in deciseconds (0 means "blink really fast") * | | ??? (Some form of phase shift or duty_length multiplier?) * | | | % of duty_length the led is off (0xff means 100%) * | | | | % of duty_length the led is on (0xff mean 100%) * | | | | | * 0xff, 0x27, 0x10, 0x00, 0x32,
(Melonee) I looked everywhere for this info I couldn't find it anywhere I've been twiddling around with this for a day or two trying to figure it out
(Chad) I think this is a good idea. I think we should sick to 0.0 to 1.0 though instead of bytes.
(Melonee) Yes you're right it should probably be 0.0-1.0
- I just found some information on the flash control part of the message:
(Melonee) okay I can see that you guys want more features/more generic msg. We should come to an agreement on what features we want and look at what the wiimote already does however many people have complained that the wiimote msgs are difficult to use. wiimote/RumbleControl and wiimote/LEDControl
(Chad) Yes, those look terribly painful. I think if we have a simple easy to use representation of LEDs and Rumble, it would be up to the driver to interpret and fulfill the messages' requests as needed. I always try to lean toward very abstract messages and with very specific drivers.
(Chad) OK, so I tried to define something that's both flexible and simple. joy/Feedback will output duration to remain on and intensity for rumble. I think having period/duty cycle/numberOfPeriods would place too much complication on the driver. However, just having a binary on/off message might be too much for the publisher. I compromised with a message that will turn on the functions for a time and then the driver will be responsible for turning them off. Sort of a "Set it and Forget it" mindset.
(Dave) I have added my offering above ("Another Possibility") I would like a protocol that makes simple things simple to do, but lets complex things happen too, without a lot of manual timing programming by the senders of the messages.
(Melonee) +1 for "Another Possibility"
(Melonee) - ping Any feedback on dave's message suggestion?
(Dave) Per conversation with Melonee, I added an array container to "Another Possibility" above so you can send an entire new configuration all at once.
(Chad) Can we add a way to provide an arbitrary number of LEDs and Rumbles? Maybe remove the strings from /Feedback. Then make /FeedbackArray have Feedback[] LEDs and Feedback[] Rumble. The disadvantage to this is that you'll have to look into the drivers to understand which index will control which LED/Rumble, but that wouldn't be more difficult than the current sensor_msgs/Joy implementation for axes and buttons and is a good way to handle different hardware.
(Melonee) okay if we create Feedback[] LEDs and Feeback[] Rumble, say you want to send LED1 blinking forever.. then later want to have LED2 blink and such.. how do you identify LED2 without resending or changing LED1
(Chad) Fixed for 'Another Possibility Refined'
Rationale for 'Another Possibility Refined'
(Chad)
The main purpose for these compound messages is to ease the work done within a publisher by allowing the publisher to ignore timing and needed loops. However, I think that these messages strongly need a support node, let's called it feedback_tools, that will take in FeedbackSequenceArray, FeedbackArray, and FeedbackSequence and output only Feedback. Lots of complicated timing in parallel will make driver development difficult and driver developers will write redundant code.
The only thing I'm willing to ask of a driver is that it accept the Feedback message and have timing to turn off the value at an approximate time. Ex: If the joystick runs at a certain rate, then I'm fine with it turning off an led at its convenience in its normal loop instead of spawning timers or threads.
If the hardware supports it, then a driver can choose to accept other messages as applicable. For instance, if there is a command to blink or rumble with certain periods/duty cycles, then the driver could choose to accept FeedbackSequence. Or if the command mandates that all LED information is sent at once, the driver can choose to accept FeedbackArray. These messages should be optional to support less work in developing simple, initial drivers.
(Melonee) I see the good of having a module or class that does timing that the driver can call but having a node that does it and then spits out feedback seems like it will change the expected behavior because then you can no longer set all the values at once and will be dependant on network latency
Also the current message doesn't allow for using the built in driver functionality because it will be almost impossible to convert the second timing to the ps3joy duty cycle input
(Tully) I feel like we're getting overly complicated in our design as we start to build in parallel and sequential execution. It seems to me that it would make more sense to use a system designed and built for these things such as SMACH and just have the driver execute the basic Feedback message, probably with support for arrays of commands. As long as you execute the script to send multiple Feedback messages on a low latency link you will have much finer grain control and we don't have to make all the drivers support different levels of feedback.
(Chad) OK, that sounds good. So drivers should support Feedback and FeedbackArray and then a SMACH node will run on the same machine as the joystick driver. One clarification is what drivers should do if an element is not included in an array: they should leave that element in its last state. This is a "send changes only" approach. If we're not greatly concerned with latency, we can also remove duration from the Feedback message; that will significantly simplify driver development.
(Tully) It's probably easier to just support FeedbackArrays since any driver can easily iterate over the incoming commands, and then there's only one topic(simplicity again). And yes, I'd expect a new message coming in to leave any unaddressed elements in its last state. I think that the simplification of removing the duration is good too, since it doesn't have a fixed starting time anyway it's already vulnerable to latency.
(Melonee) I am good with the above proposal to make a ~set_feedback topic that takes FeedbackArray without duration and create SMACH nodes to do the rest. Are we all in agreement?
(Chad) I think so! I tried a quick trial run with the WiiMote. The driver implementation was very simple and easy! I didn't run into any errors, so I'll confirm that this looks like a usable solution.
(Tully) I'm ok with that except for the private topic.
(Melonee) okay I will make the topic joy/set_feedback so it can be remapped easily and put the msg in sensor_msgs as JoyFeedbackArray and JoyFeedback
(Tully) I think this is good solution.
(Melonee) implemented in trunk r37418 and example documentation on the ps3joy main page
Meeting agenda
To be filled out by proposer based on comments gathered during API review period
Conclusion
Package status change mark change manifest)
![/!\](/moin_static197/rostheme/img/alert.png "/!\")
Action items that need to be taken. 
Major issues that need to be resolved
