Feat/can node #13
Feat/can node #13
Conversation
…, estop, mob_stop, and mob_start into can_node
| self.safetyLightsSubscriber = self.create_subscription( | ||
| SafetyLights, | ||
| "autonav/safety_lights", | ||
| self.on_safety_lights_received(), |
There was a problem hiding this comment.
remove parentheses (and on following lines) as this is calling the function instead of passing the function as a reference python thingamajig
|
|
||
| def init(self): | ||
| # can threading | ||
| self.canTimer = self.create_timer(0.5, self.can_worker) |
There was a problem hiding this comment.
is the .5 a timeout and should it be shorter or longer? half a second seems slow for CAN stuff but idk what the number means
There was a problem hiding this comment.
^ this is attempting to make a new can worker if there isn't one already every .5 seconds, seems reasonable
| class CanConfig: | ||
| def __init__(self): | ||
| self.odom_feedback_scaler = 10000 | ||
| self.linear_pid_scaling_factor = 1000 |
There was a problem hiding this comment.
for consistency they should all be scaler or all be factor, I don't like having both right next to each other
|
|
||
| def can_worker(self): | ||
| try: | ||
| with open("/dev/ttyACM0", "r") as f: |
There was a problem hiding this comment.
can we add an easy way to change the device path? I think in the future we'll make a udev rule for it and make it like ttyACM200 or something because the first like 80 are auto-assigned, might be nice to check multiple paths so we don't accidentally miss it or something
| def can_worker(self): | ||
| try: | ||
| with open("/dev/ttyACM0", "r") as f: | ||
| pass |
There was a problem hiding this comment.
wait why are we just passing here?
| if arbitration_id == arbitration_ids["Estop"]: | ||
| self.set_mobility(False) | ||
|
|
||
| if arbitration_id == arbitration_ids["MobilityStart"]: |
There was a problem hiding this comment.
not a super huge thing but this could be optimized if it's one big if/elif chain instead of multiple individual if statements
There was a problem hiding this comment.
plus we can throw an else: at the end of it all and throw an error because we should never get an ID that's not one of those listed here
| forward_velocity, forward_velocity_setpoint, sideways_velocity, sideways_velocity_setpoint = struct.unpack("hhhh", msg.data) | ||
| linear_pid_statistics_msg = LinearPIDStatistics() | ||
| linear_pid_statistics_msg.forward_velocity = forward_velocity | ||
| linear_pid_statistics_msg.forward_velocity_setpoint - forward_velocity_setpoint |
There was a problem hiding this comment.
probably want this to be an equal sign instead of a minus sign
| try: | ||
| self.can.send(can_msg) | ||
| except can.CanError: | ||
| pass |
There was a problem hiding this comment.
do we want to be passing these can errors or should the user know about them? how often do they occur? is it like a once every couple seconds because communication protocols be like that or is it a big deal and we should change device state and log an error?
| "msg/Log.msg" | ||
| "msg/MotorFeedback.msg" | ||
| "msg/MotorInput.msg" | ||
| "msg/MotorStatisticsBackMotors.msg" | ||
| "msg/MotorStatisticsFrontMotors.msg" |
There was a problem hiding this comment.
why are these separate messages? aren't both back and front motor messages the same? they should be different callbacks yes but same message type
| uint8 left_drive_motor_output | ||
| uint8 left_steer_motor_output | ||
| uint8 right_drive_motor_output | ||
| uint8 right_steer_motor_output |
There was a problem hiding this comment.
see comment about separate front and back message types
New Can node based on new can spec.