-
Notifications
You must be signed in to change notification settings - Fork 60
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
ROS Noetic, Python 3 Updates #31
base: master
Are you sure you want to change the base?
ROS Noetic, Python 3 Updates #31
Conversation
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.
Hello! I was looking for someone who made respeaker_ros work in python3 and found this PR. I did a quick review and asked some questions and I am sorry if they are unclear or sound rude, I find this PR very helpful and just want to contribute!
Mirko
scripts/speech_to_text.py
Outdated
@@ -15,10 +15,10 @@ | |||
class SpeechToText(object): | |||
def __init__(self): | |||
# format of input audio data | |||
self.sample_rate = rospy.get_param("~sample_rate", 16000) | |||
self.sample_width = rospy.get_param("~sample_width", 2) | |||
self.sample_rate = 16000 #rospy.get_param("~sample_rate", 16000) |
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 remove parameter? is this a leftover?
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.
not sure why I hardcoded this, maybe I was changing the rate during debugging
@@ -41,7 +41,7 @@ def __init__(self): | |||
|
|||
self.pub_speech = rospy.Publisher( | |||
"speech_to_text", SpeechRecognitionCandidates, queue_size=1) | |||
self.sub_audio = rospy.Subscriber("audio", AudioData, self.audio_cb) | |||
self.sub_audio = rospy.Subscriber("speech_audio", AudioData, self.audio_cb) |
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.
public topic name change, meaning API changed. intended?
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.
yes, this is important, the package appears to be set up to publish raw audio to "audio" and processed, clean audio to "speech_audio", so the speech to text feature ought to use the cleaned up audio for better recognition
@@ -60,23 +60,31 @@ def tts_timer_cb(self, event): | |||
self.last_tts = stamp | |||
if stamp - self.last_tts > self.tts_tolerance: | |||
rospy.logdebug("END CANCELLATION") | |||
self.is_canceling = False | |||
self.is_canceling = Falser |
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.
typo?
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.
yes
scripts/speech_to_text.py
Outdated
|
||
def audio_cb(self, msg): | ||
if self.is_canceling: | ||
rospy.loginfo("Speech is cancelled") | ||
return | ||
data = SR.AudioData(msg.data, self.sample_rate, self.sample_width) | ||
data = SR.AudioData(bytes(msg.data), self.sample_rate, self.sample_width) | ||
with open(str(len(msg.data)) + ".wav","wb") as f: |
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.
change of behavior by writing to file, I think this is not related to python3
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.
casting msg.data to bytes before sending to AudioData is indeed related to python3. opening the wave file is just from debugging. again, sorry I should have cleaned this up.
result = self.recognizer.recognize_google( | ||
data, language=self.language) | ||
msg = SpeechRecognitionCandidates(transcript=[result]) | ||
result = self.recognizer.recognize_sphinx( |
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.
not an expert on this, but google->sphinx may change user experience somehow
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 should be removed now. specific to my use case
scripts/respeaker_node.py
Outdated
@@ -377,15 +397,19 @@ def on_status_led(self, msg): | |||
oneshot=True) | |||
|
|||
def on_audio(self, data, channel): | |||
|
|||
if channel == 0: |
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.
?
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.
also just debugging
scripts/respeaker_node.py
Outdated
self.respeaker_audio.start() | ||
self.info_timer = rospy.Timer(rospy.Duration(1.0 / self.update_rate), | ||
self.on_timer) | ||
self.timer_led = None | ||
self.sub_led = rospy.Subscriber("status_led", ColorRGBA, self.on_status_led) | ||
self.big_data0 = [] | ||
self.out = wave.open("/home/pi/Desktop/test.wav", 'wb') |
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.
is this self.out some debug code? hardcoded path will not work anywhere else
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.
yes debug code, sorry!
Hi, glad I could help! Apologies it seems I forgot to clean up my code before making the PR. There are also some changes specific to my use case in here that I forgot to make a new branch for. I just reset the branch to before that commit and also removed some of the debugging lines. Still have to test. I'll respond to your comments directly as well. Let me know if you have any more questions! |
e73482b
to
415fb74
Compare
Hey, File "/opt/ros/noetic/lib/python3/dist-packages/tf2_py/init.py", line 38, in File "/opt/ros/noetic/lib/python3/dist-packages/rospy/impl/tcpros_base.py", line 167 Can you may help me fixing this?! Thanks so much |
Hi,
Happy to help but the lines you listed don't exist in the respeaker_ros
code. Either the errors are with your ros installation or you haven't given
the relevant lines from the error messages.
Best,
Zach
…On Fri, Sep 2, 2022, 6:06 AM JohannaPrinz ***@***.***> wrote:
Hey,
thanks for debugging the code, so it works with ROS Noetic and Python3
I have applied all changes, but still get the following error messages:
1.
from ._tf2 import *
ImportError: dynamic module does not define init function (init_tf2)
1.
(e_errno, msg, *_) = e.args
^
SyntaxError: invalid syntax
Can you may help me fixing this?! Thanks so much
Best regards
—
Reply to this email directly, view it on GitHub
<#31 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/APSFFUDGB4QIQY7SIWEGE7TV4HGS3ANCNFSM5USORCZQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
@@ -1,6 +1,5 @@ | |||
PyAudio==0.2.8 | |||
SpeechRecognition==3.8.1 | |||
click==6.7 | |||
numpy==1.16.2 |
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.
Are you sure? I believe numpy is still required. See scripts/respeaker_node.py line 11
self.bitdepth = 16 | ||
self.i = 0 |
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 also a debugging left-over?
@@ -7,6 +7,7 @@ | |||
import usb.core | |||
import usb.util | |||
import pyaudio | |||
import wave |
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 should be removed again as well, right? that was for the wav file you used for debugging?
All correct, thank you!
…On Thu, Sep 8, 2022, 4:15 AM Severin Fichtl ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In scripts/respeaker_node.py
<#31 (comment)>
:
> @@ -7,6 +7,7 @@
import usb.core
import usb.util
import pyaudio
+import wave
this should be removed again as well, right? that was for the wav file you
used for debugging?
—
Reply to this email directly, view it on GitHub
<#31 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/APSFFUFUCUHKRVNBZD2NWBDV5GOC5ANCNFSM5USORCZQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Hi @zacharykratochvil, thank you for creating this PR! I gave it a try and it needed a few fixes to get past |
After installing on RaspberryPi 4B, Ubuntu Impish, ROS 1 Noetic, Python 3.9.7, the Respeaker array would not initialize. It required two additional dependencies not listed in package.xml and had old python 2 syntax and function calls in respeaker_node.py. After these corrections it initializes and runs well. I believe these changes are necessary for anyone running newer versions of python and ROS so others might benefit from this branch being publicly available. Not sure about their backwards compatibility though, so it may be best to leave them in a separate branch.