Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed command to IntEnum. Fixes issue #934

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 28 additions & 10 deletions mil_common/drivers/sabertooth2x12/sabertooth2x12/board.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
#!/usr/bin/env python3
import struct
from enum import IntEnum

import serial

from .simulated import SimulatedSabertooth2x12


class CommandEnum(IntEnum):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new class you just made needs to be added to the documentation as an enumeration.

MOTOR1_FORWARD
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't do this sort of thing. Check out my comment below first if you haven't seen that. In this case, there are definitely integer values that each keyword should be associated with.

MOTOR1_BACKWARD
MOTOR2_FORWARD
MOTOR2_BACKWARD


class Sabertooth2x12:
"""
Helper class to interface with the Sabertooth 2x12 regenerative motor driver.
Expand Down Expand Up @@ -38,29 +46,39 @@ def __init__(
self.ser = SimulatedSabertooth2x12()

@staticmethod
def make_packet(address: int, command: int, data: int) -> bytes:
def make_packet(address: int, command: CommandEnum, data: int) -> bytes:
"""
Constructs a packet given an address, command, and data. The checksum is
added on to the end of the packet. All four integers are packed as unsigned
integers in the resulting packet.

Args:
address (int): ???
command (int): The command to send.
command (CommandEnum): The command to send.
data (int): The data to put in the packet.

Returns:
bytes: The constructed packet.
"""
checksum = (address + command + data) & 127
return struct.pack("BBBB", address, command, data, checksum)
intCommand = 0
if command == CommandEnum.MOTOR1_FORWARDS:
intCommand = 0
elif command == CommandEnum.MOTOR2_FORWARDS:
intCommand = 4
elif command == CommandEnum.MOTOR1_BACKWARDS:
intCommand = 1
elif command == CommandEnum.MOTOR2_BACKWARDS:
intCommand = 5
Comment on lines +63 to +71
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this isn't how enums are meant to be used.

You should try a design pattern like this:

>>> from enum import IntEnum
>>> class Fruit(IntEnum):
>>>     APPLE = 1
>>>     BANANA = 3
>>>     ORANGE = 43
>>> print(f"The value of Fruit.ORANGE is {Fruit.ORANGE.value}!")
The value of Fruit.ORANGE is 43!

As I tried to express above, use the values that enums hold to your advantage. Don't use enums just because they allow you to use words in the program. Set these words equal to a value, and then use the value of this enumeration throughout your program.

Feel free to look up more info about enums and their use, or reach out, and I'd be glad to help! :)


checksum = (address + intCommand + data) & 127
return struct.pack("BBBB", address, intCommand, data, checksum)

def send_packet(self, command: int, data: int) -> None:
def send_packet(self, command: CommandEnum, data: int) -> None:
"""
Sends a packet over the serial connection using the class' address.

Args:
command (int): The command to send.
command (Command): The command to send.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo

data (int): The data to put in the packet.
"""
packet = self.make_packet(self.address, command, data)
Expand All @@ -75,9 +93,9 @@ def set_motor1(self, speed: float) -> None:
speed (float): The speed to set the first motor to.
"""
if speed < 0:
command = 1
command = CommandEnum.MOTOR1_BACKWARDS
else:
command = 0
command = CommandEnum.MOTOR1_FORWARDS
data = int(min(1.0, abs(speed)) * 127)
self.send_packet(command, data)

Expand All @@ -90,8 +108,8 @@ def set_motor2(self, speed: float) -> None:
speed (float): The speed to set the second motor to.
"""
if speed < 0:
command = 5
command = CommandEnum.MOTOR2_BACKWARDS
else:
command = 4
command = CommandEnum.MOTOR2_FORWARDS
data = int(min(1.0, abs(speed)) * 127)
self.send_packet(command, data)