-
Notifications
You must be signed in to change notification settings - Fork 394
Conversation
d9b35a7
to
167e001
Compare
So I gave this a lot of testing and fixing so it should hopefully be okay. @alexa-pi/coreteam Please test and confirm, if it's okay or not. Thanks! |
/usr/bin/python2 $@ | ||
} | ||
|
||
function run_pip { |
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.
Only "install" pip command is used. It would it be cleaner to rename run_pip
to pip_install
and change line 12 to:
$(which pip2) install $@
On my RPI2 and RPI3 pip2 is in /usr/local/bin
, so it is better to not hardcode /usr/bin
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.
Good point there! Changed to the which stuff 😄
I'm keeping the run_ instead of install_, since now we actually use it and I have a hunch that we might use it for other things also.
Thanks!
548a279
to
56c75fb
Compare
I've installed and run this successfully on a Pi running Raspbian, and I got it to work on a CHIP too. For the CHIP, I still had to: 1.) edit /etc/opt/AlexaPi/config.yaml to change device from "raspberrypi" to "chip" It is slightly misleading to ask the user what device they are using then not have it reflected in the imported config.yaml file. How hard would it be to change that during install? Also, since it isn't running as root out of the gates, install doesn't "just work". I'm assuming that if a CHIP user managed to get that far they could figure it out, but what are thoughts on making root a default for CHIP and Orange Pi since we know we need it right now anyway? |
This is all intentional, since the root platforms should be discouraged generally as insecure and some people might use the dummy platform when they don't need GPIO and the rest should be warned before using it. But obviously I didn't think this through, you are right! Not even telling the user is not much of a warning. Also, broken-by-default is stupid. Thanks for pointing this all out! So I'm gonna:
Sounds okay? |
For 3, perhaps cleaner way of doing that is to separate CHIP/OrangePi root override into a separate script with "on/off" switch? So security minded people may want to switch button/LED off once they tried them out. |
So I've been thinking about it for the last hour and I still don't feel very keen about doing it since it's just two commands (cp/rm & BTW, I've found https://github.com/FlavourSys/bash-installer-framework which is something we might use in the future - a lot has happened here so let's just finish it as it was designed and do other improvements in other PRs. |
I am all for the "on/offing" things in the future. After the issue on the HiFiBerry GPIO conflict last night (also with TFT screens, other hats), I think a modular "device announced" approach makes more sense. Any thoughts on what I wrote up a while back? |
@renekliment I think it might be worthwhile to have something to inform the user about next steps--maybe a string of text during the install about running as root if CHIP or Orange Pi has been selected. And either include a blurb there about modifying the "device" in config.yaml or adjust it automatically. However, I know there are bigger changes in the works, and if they are planned to help remove some of these stumbling blocks I'm okay with merging this and moving on. Realistically, this stuff is in the wiki. There's nothing wrong with the PR -- just things that seemed like good additions as I was installing it. |
@maso27 Don't get me wrong, you have fair points there. We're totally including the stuff in the setup script as mentioned above. |
@illperipherals Feel free to open a PR. I don't have it on my todo / priority list. |
Alright, so:
@alexa-pi/coreteam So since this is only a step forward and now it shouldn't confuse OPi / CHIP users, I'm for merging this if there are no other issues. Please test and tell me if it's okay to go. The automatic configuration of root-only platforms is something someone else could take up. It should be fairly easy, since we have the |
Works with CHIP! |
Great! Merging this and hoping I can finish the other PR soon, so we can make a new release and take a vacation :-) Thank you for testing Mason! And also for the Arch Linux stuff! |
Also, anyone feel free to take the automatic root platform setup, it's easy. |
Huh. I guess I should've tested the Raspberry Pi again, instead of just focusing on the CHIP with #82. Sorry folks! |
Fixes #60, #79.
What's still missing:
Fedora support - does anyone use that? Do we care?Can be easily added later when there are people to use that on daily basis.
Exec=/usr/bin/python2 ...
${monitorAlexa}
as booleanNeed to do:
This enables on-board audio via Device Trees.
Probably later: