-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: add limo speaker package #6
Conversation
Oops! |
Didn't work, sounddevice module not installed... code-server not openable, have i broken something badly?
and
running |
Currently facing an issue where code-server will not work on anything other than the LIMO, this is an upstream issue from code-server themselves, see issue coder/code-server#7219 This is working fine on the LIMOs back screen and if I open an ssh tunnel to the limo, so the page is served over a secure context, then its fine?! ie: ssh -L 9999:localhost:9999 [email protected] |
Doing this to quickly test if it merges and works... preventing issues from building up
… make the temp file safely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I conditionally approve, assuming you have tested it works.
# this is using tempfile to make a safe file name | ||
# e.g. tts_a1b2c3.wav | ||
temp = tempfile.NamedTemporaryFile(prefix="tts_", suffix=".wav") | ||
file_path = temp.name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not quite how it should be used, I understand, see https://docs.python.org/3/library/tempfile.html#tempfile.NamedTemporaryFile
I think in your case https://docs.python.org/3/library/tempfile.html#tempfile.mkstemp may be better used, but if it works for you and you have tested this, I'm happy to approve.
Still working on it, currently working on getting the .wav files copied over into the share directory |
…file usage in playAudio and playTTS
To get sound you have to bring it up with
|
If there are no side effect, I can't see why we wouldn't want to launch it. |
RUN curl -fsSL https://code-server.dev/install.sh | sh | ||
|
||
USER ros |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why has this been removed? This changes the final image as it leaves it with root user. Is it intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooops, I'll add that back… Was modifying it when I was debugging the issues with packages installed into the path.
This line, adds it to the compose hence it was missed.
limo_platform/configs/docker-compose.yaml
Line 42 in c8b93e3
user: "ros" |
What type of PR is this? (check all applicable)
Description
Adds a car horn and text to speech support for the LIMO platform. This allows for simple use of the speakers for student projects etc.
Related Tickets & Documents
QA Instructions, Screenshots, Recordings
Please replace this line with instructions on how to test your changes.
Limo should launch the nodes
/speaker/play
and/speaker/tts
with the launch fileYou can then use the following lines in the code-server instance to publish the messages.
and also
[optional] Are there any post deployment tasks we need to perform?
Before you merge this branch, a PR needs to be raised against the
limo_ros2
repo to merge in josh/car-horn and then this PR updated to have the correct submodule ref.