Add instructions for setting up on Raspbian in voicekit branch. #149

t1m0thyj · 2017-10-12T15:31:35Z

This PR is my attempt to fix #148. I've added back the files HACKING.md and systemd/voice-recognizer.service which were in the master branch. I also updated the instructions in HACKING.md to work with the voicekit branch.

IMO these files are too useful to not be present in the new branch, and having them there doesn't change the way that this project behaves for anyone who doesn't want to use them.

The way the install-services.sh script is currently written, it will automatically add any services in the systemd folder to the /lib/systemd/system folder on the Pi, including voice-recognizer.service if it is present. It will also replace any references to /home/pi in the service file, updating them to the correct installation folder. However, the service will never run unless it is explicitly enabled by the user.

t1m0thyj · 2017-10-13T12:27:57Z

I intended to create a second pull request for adding the IP address action, but that commit got combined with this one. (Sorry, I'm fairly new to using GitHub.)

@drigz commented on #138 that a PR to add this action would be welcome. There was a PR to add this a few days ago that was killed by the submitter, so I thought I would try to revive it. I initially had issues getting the "ip address" command to work, but capitalizing "IP" made Assistant recognize it.

I also added "raspberry" in front of the "power off" and "reboot" commands, and fixed the indentation of the power_off_pi() and reboot_pi() functions (changed it from tabs to spaces to be consistent and follow PEP8).

divx118 · 2017-10-14T07:46:14Z

@t1m0thyj FYI to make separate pull requests and also in general before working on a forked repo it is better to make your own working branches. You can do this very easily through the website of github. See https://github.com/blog/1377-create-and-delete-branches.
Note that before you type in the new name of your branch switch to the branch that you want to duplicate. Then make the changes and create a pull request. Like in your case you could have better make 2 duplicates of the voicekit branch. Use one for the files to add for setting up AIY on raspbian. The second one to make the changes to the demo. So you can make 2 separate pull requests.
BTW when string checking you could use str.lower() this would avoid problems when the assistent server returns capital letters. So text.lower() == 'ip address'

t1m0thyj · 2017-10-16T02:43:30Z

@divx118 Should I just do that in the future, or do you want me to close this PR and create new ones in the way you described?

drigz

Thanks for the contribution, @t1m0thyj!

drigz · 2017-10-18T08:43:42Z

HACKING.md

+
+## Installing the Voice HAT driver and config
+
+To use the Voice HAT, you'll need to upgrade your kernel to 4.9, then adjust the


This is no longer true as Raspbian ships with 4.9 since 2017-07-05. Please remove the apt-get commands and add to the text something like "Your kernel needs to be 4.9 or later. This is available on Raspbian 2017-07-05 and later."

drigz · 2017-10-18T08:47:07Z

HACKING.md

+sudo systemctl enable voice-recognizer.service
+```
+
+# I18N


Please remove the I18N section, since the samples don't contain internationalized code.

drigz · 2017-10-18T08:47:22Z

src/assistant_library_with_local_commands_demo.py

@@ -50,6 +50,11 @@ def reboot_pi():
    subprocess.call('sudo reboot', shell=True)


+def say_ip():


Please split the addition of this command out into a separate PR. As @divx118 says, it's much better to split changes up - then one can be merged even if the other is still being reviewed.

drigz · 2017-10-18T08:51:14Z

HACKING.md

+[setup instructions](https://aiyprojects.withgoogle.com/voice#users-guide-1-1--connect-to-google-cloud-platform) on the
+webpage.
+
+# Making code changes


This heading doesn't make sense - not all of the following text should be under "Making code changes". Maybe just add "## Running automatically" before the service bit.

I wasn't sure whether there should be 1 or two hash signs before "Making code changes" to make it either a level 1 or a level 2 header. I changed it to two, because I didn't think "Running automatically" belonged under it.

drigz · 2017-10-18T08:54:21Z

src/assistant_library_with_local_commands_demo.py

            assistant.stop_conversation()
            power_off_pi()
-        elif text == 'reboot':
+        elif text == 'raspberry reboot':


Please revert the addition of "raspberry". Since this is just a sample and everyone has different preferences, I prefer to keep it simple.

t1m0thyj · 2017-10-18T12:40:47Z

Removed IP address action from this PR and updated HACKING.md based on feedback.

drigz · 2017-10-18T12:53:22Z

HACKING.md

+sudo scripts/install-services.sh
+```
+
+## Installing the Voice HAT driver and config


Can you also change this header to "Configuring the Voice HAT driver" since we're not installing now?

drigz · 2017-10-18T15:27:31Z

Excellent, thanks so much @t1m0thyj!

googlebot added the cla: yes label Oct 12, 2017

t1m0thyj force-pushed the voicekit branch from f1f70d3 to 885b35f Compare October 16, 2017 14:26

drigz reviewed Oct 18, 2017

View reviewed changes

t1m0thyj force-pushed the voicekit branch from 4fed793 to 13e59cc Compare October 18, 2017 12:06

t1m0thyj added 2 commits October 18, 2017 08:36

Add instructions for setting up on Raspbian in voicekit branch.

aadc57c

Update HACKING.md based on feedback from drigz

d00749d

t1m0thyj force-pushed the voicekit branch from 13c853f to d00749d Compare October 18, 2017 12:36

drigz reviewed Oct 18, 2017

View reviewed changes

t1m0thyj mentioned this pull request Oct 18, 2017

Added "IP address" action that says Pi's IP #157

Merged

Changed header in HACKING.md for Voice HAT driver

227e9ad

drigz merged commit 79f1dd6 into google:voicekit Oct 18, 2017

t1m0thyj mentioned this pull request Oct 23, 2017

Add mention of HACKING.md to readme #162

Merged

RichardBronosky mentioned this pull request Jan 1, 2018

install-deps.sh has hardcoded project path in conflict with instructions in HACKING.md #211

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add instructions for setting up on Raspbian in voicekit branch. #149

Add instructions for setting up on Raspbian in voicekit branch. #149

t1m0thyj commented Oct 12, 2017 •

edited

Loading

t1m0thyj commented Oct 13, 2017 •

edited

Loading

divx118 commented Oct 14, 2017

t1m0thyj commented Oct 16, 2017 •

edited

Loading

drigz left a comment

drigz Oct 18, 2017

drigz Oct 18, 2017

drigz Oct 18, 2017

drigz Oct 18, 2017

t1m0thyj Oct 18, 2017

drigz Oct 18, 2017

t1m0thyj commented Oct 18, 2017 •

edited

Loading

drigz Oct 18, 2017

drigz commented Oct 18, 2017


		## Installing the Voice HAT driver and config

		To use the Voice HAT, you'll need to upgrade your kernel to 4.9, then adjust the

		@@ -50,6 +50,11 @@ def reboot_pi():
		subprocess.call('sudo reboot', shell=True)


		def say_ip():

Add instructions for setting up on Raspbian in voicekit branch. #149

Add instructions for setting up on Raspbian in voicekit branch. #149

Conversation

t1m0thyj commented Oct 12, 2017 • edited Loading

t1m0thyj commented Oct 13, 2017 • edited Loading

divx118 commented Oct 14, 2017

t1m0thyj commented Oct 16, 2017 • edited Loading

drigz left a comment

Choose a reason for hiding this comment

drigz Oct 18, 2017

Choose a reason for hiding this comment

drigz Oct 18, 2017

Choose a reason for hiding this comment

drigz Oct 18, 2017

Choose a reason for hiding this comment

drigz Oct 18, 2017

Choose a reason for hiding this comment

t1m0thyj Oct 18, 2017

Choose a reason for hiding this comment

drigz Oct 18, 2017

Choose a reason for hiding this comment

t1m0thyj commented Oct 18, 2017 • edited Loading

drigz Oct 18, 2017

Choose a reason for hiding this comment

drigz commented Oct 18, 2017

t1m0thyj commented Oct 12, 2017 •

edited

Loading

t1m0thyj commented Oct 13, 2017 •

edited

Loading

t1m0thyj commented Oct 16, 2017 •

edited

Loading

t1m0thyj commented Oct 18, 2017 •

edited

Loading