Summary

There are inconsistencies in naming of different HeadMode patterns, between the HeadMode.msg and the config of the bitbots_head_mover.

Expected behavior

I would expect patterns to have a descriptive and consistent name.

Current behavior

# This message is used for communicating between the body behavior and the head behavior.
# The body tells the head by this message what it shall do.

# Search for Ball and track it if found
uint8 BALL_MODE=0 
# Look generally for all features on the field (ball, goals, corners, center point)
uint8 FIELD_FEATURES=1
# Simply look directly forward
uint8 LOOK_FORWARD=2
#Don't move the head
uint8 DONT_MOVE=3
# Ball Mode adapted for Penalty Kick
uint8 BALL_MODE_PENALTY = 4
# Do a pattern which only looks in front of the robot
uint8 LOOK_FRONT = 5

uint8 head_mode

• HeadMode 0 (BALL_MODE) corresponds to search_pattern in the config
• HeadMode 1 (FIELD_FEATURES) is name search_pattern_field_features
• HeadMode 2 (LOOK_FORWARD) corresponds to look_forward in the config
• HeadMode 3 (DONT_MOVE) could maybe be look_at in config?
• HeadMode 4 (BALL_MODE_PENALTY) corresponds to search_pattern_penalty
• HeadMode 5 (LOOK_FRONT) corresponds to front_search_pattern in config

Suggested Changes

• HeadMode 0 (BALL_MODE)
  • BALL_MODE does not explain, that we are searching for the ball and could be confused with LOOK_FRONT which only moves the head up and down once the ball has been found (I think?). Also, MODE seems unnecessary, as others also don't contain it. -> could be HeadMode.SEARCH_BALL
  • search_pattern gives no explanation at all -> could be search_pattern_ball
• HeadMode 1 (FIELD_FEATURES)
  • FIELD_FEATURES -> could be HeadMode.SEARCH_FIELD_FEATURES to better differentiate from other modes, where we already found the target to look at and for consistency with the search_pattern_field_features
• HeadMode 3 (DONT_MOVE)
  • if look_at in deed corresponds to no movement and instead looking at a specific point, without a pattern I think it should be renamed -> could be HeadMode.LOOK_AT or config name change to dont_move depending on the actual behavior
• HeadMode 4 (BALL_MODE_PENALTY)
  • -> could be HeadMode.SEARCH_BALL_PENALTY for most consistency
  • search_pattern_penalty then makes sense to change as well **-> could be search_pattern_ball_penalty
• HeadMode 5 (LOOK_FRONT)
  • LOOK_FRONT seems to be easily confusable with LOOK_FORWARD and since it is not only looking but also a "search pattern" -> could be HeadMode.SEARCH_FRONT
  • front_search_pattern for consistency in config -> could be search_pattern_front

If changing the config anyway, it might make sense to adjust the structure to be able to use the exact same name as in the ros message for easier searching within the monorepo.
This could be done by adjusting the structure by introducing a movement_patterns key:

movement_patterns:
  search_ball: # HeadMode 0
  ...
  search_field_features: # HeadMode 1
  ...
  look_forward: # HeadMode 2
  ...
  dont_move: # HeadMode 3
  ...
  search_ball_penalty: # HeadMode 4
  ...
  search_front: # HeadMode 5