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

audible feedback #15

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

audible feedback #15

wants to merge 13 commits into from

Conversation

antonioc76
Copy link
Member

Will play audio files sent in an AudibleFeedback message.

AudibleFeedback message also contains a field which if True will stop all currently playing tracks.

@danielbrownmsm danielbrownmsm self-requested a review November 25, 2024 23:47
@danielbrownmsm danielbrownmsm added the enhancement New feature or request label Nov 25, 2024
Copy link
Collaborator

@danielbrownmsm danielbrownmsm left a comment

Choose a reason for hiding this comment

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

looks good to me, although I want to get feat/ros-unity-integration merged first as its own PR so that it isn't part of this PR to keep things more separated so waiting on merging this for a bit

<package format="3">
<name>autonav_hardware</name>
<version>0.0.0</version>
<description>TODO: Package description</description>
Copy link
Collaborator

Choose a reason for hiding this comment

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

version 2025.0.1 (or .1.0, whichever you prefer) and package description

20
)

self.system_state_monitor = self.create_timer(0.5, self.monitor_system_state)
Copy link
Collaborator

Choose a reason for hiding this comment

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

SystemStateTransition might get implemented this year, TBD, so you might be able to change this in the future

def on_audible_feedback_received(self, msg:AudibleFeedback):
self.monitor_tracks()

self.log(f"{len(self.tracks)}", LogLevel.ERROR)
Copy link
Collaborator

Choose a reason for hiding this comment

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

change to LogLevel.DEBUG before merging so no red ERROR color jumpscares


def play_sound(self, filename):
playback = Playback()
playback.load_file(filename)
Copy link
Collaborator

Choose a reason for hiding this comment

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

if this file doesn't exist does this fail gracefully or crash the code? if it crashes a try/catch would be nice, otherwise no problems, although doing some processing on the file name (like a .strip() maybe) might be worth it?

…use buttons to controller dpad, and secondary tracks example to controllre X button
@danielbrownmsm danielbrownmsm self-requested a review November 30, 2024 04:33
Copy link
Collaborator

@danielbrownmsm danielbrownmsm left a comment

Choose a reason for hiding this comment

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

looks good to me

audible_feedback.pause_all = True
self.audibleFeedbackPublisher.publish(audible_feedback)

if self.controller_state['abs_hat0x'] == 1:
Copy link
Collaborator

Choose a reason for hiding this comment

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

could be an elif instead of an if but no biggie

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants