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

Change button colors on arm and disarm #99

Merged
merged 28 commits into from
Nov 16, 2024
Merged

Conversation

ShannonGriswold
Copy link
Contributor

Pull Request

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

Change the colors of the arm and disarm buttons depending on whether the robot is armed or disarmed

Related Tickets & Documents

Copy link
Contributor

@benjaminwp18 benjaminwp18 left a comment

Choose a reason for hiding this comment

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

This works great in simulation 🎉

But... I don't like how it only applies to the Arm widget. The Arm widget uses two ButtonIndicators for its buttons, which inherit from our custom IndicatorMixin widget class. IndicatorMixin is also inherited by CircleIndicator, which made pretty green/red circles for statuses like flooding & Pi connection.

CircleIndicator used broken code that switched between stylesheets that are defined in surface/gui/gui/styles (the .qss files). I kinda like your solution (defining the stylesheets in code and switching between them just by setting the stylesheet) better, but I want your solution to work for everything that inherits from IndicatorMixin. Can you update this to work from IndicatorMixin?


# Removes any state
_NO_STATE = ''
def set_initial_stylesheet(self) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Write docstrings for all the methods here
Use the VSCode autodocstring extension with the numpy preset

@@ -14,7 +14,7 @@ class Arm(QWidget):
DISARM_REQUEST = CommandBool.Request(value=False)
BUTTON_WIDTH = 120
BUTTON_HEIGHT = 60
BUTTON_STYLESHEET = 'QPushButton { font-size: 20px; }'
BUTTON_STYLESHEET = 'QPushButton { font-size: 20px;}'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
BUTTON_STYLESHEET = 'QPushButton { font-size: 20px;}'
BUTTON_STYLESHEET = 'QPushButton { font-size: 20px; }'

# Removes any state
_NO_STATE = ''
def set_initial_stylesheet(self) -> None:
self._ORIGINAL_STYLESHEET = self.styleSheet()
Copy link
Contributor

Choose a reason for hiding this comment

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

SCREAMING_SNAKE is for constants, make this self._original_stylesheet

Comment on lines 59 to 65
if self.current_state == WidgetState.ON:
return self._ON_STYLESHEET
if self.current_state == WidgetState.OFF:
return self._OFF_STYLESHEET
if self.current_state == WidgetState.INACTIVE:
return self._INACTIVE_STYLESHEET
return ''
Copy link
Contributor

Choose a reason for hiding this comment

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

Store _ON_STYLESHEET etc. in a dictionary of WidgetStates -> stylesheets

def set_inactive(self) -> None:
self.setProperty(IndicatorMixin._PROPERTY_NAME, IndicatorMixin._INACTIVE)
self._update_style()
class WidgetState(Enum):
Copy link
Member

Choose a reason for hiding this comment

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

use IntEnum

Copy link
Contributor

@benjaminwp18 benjaminwp18 left a comment

Choose a reason for hiding this comment

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

🥳

@ShannonGriswold ShannonGriswold merged commit 8941d8c into main Nov 16, 2024
2 checks passed
@ShannonGriswold ShannonGriswold deleted the color-arm-disarm branch November 16, 2024 19:02
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.

[BUG] Arm/Disarm buttons don't give feedback
3 participants