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

Conversation

OrganomagnesiumHalide
Copy link
Contributor

Description

I set the command param in the Sabertooth 2x12 class to take a CommandEnum. It goes back to using integers when getting packed into a byte

Screenshot or Video

Related Issues

#675

About This PR

  • [x ] I have updated documentation related to this change so that future members are aware of the changes I've made.

Copy link
Member

@cbrxyz cbrxyz left a comment

Choose a reason for hiding this comment

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

Good start. Check out my proposed changes for an idea of where might be good to go next.

Comment on lines +63 to +71
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
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! :)

"""
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


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.


import serial

from .simulated import SimulatedSabertooth2x12


class CommandEnum(IntEnum):
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.

@uf-mil-bot
Copy link
Collaborator

Hello, it's your friendly InvestiGator bot here!

The docs preview for this PR is available at https://uf-mil.github.io/pr-docs/934.

Last updated at:
► fe2feef3e95d291fa3b75ba99ab92bf221aa0fa2
► 2023-02-21 22:14 EST

Have a great day! Go gators! 🐊

@cbrxyz
Copy link
Member

cbrxyz commented Mar 5, 2024

Change was not finished, closing for now

@cbrxyz cbrxyz closed this Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants