Skip to content
This repository was archived by the owner on Dec 20, 2023. It is now read-only.

Setup script refactor #82

Merged
merged 16 commits into from
Nov 27, 2016
Merged

Conversation

renekliment
Copy link
Member

@renekliment renekliment commented Nov 9, 2016

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.
  • Arch Linux: "python" => "python2" everywhere (including all other scripts) - setup should be fine with the alias now - we'll add choice for py version later; do a systemd unit override for this - something like Exec=/usr/bin/python2 ...
  • include frequently used systemd unit overrides (usergroup-root and stuff)
  • OS / device detection / selection - let's just ask the user and don't over-complicate things
  • pass ${monitorAlexa} as boolean
  • config handling cleanup / refactor

Need to do:

  • once this is merged, edit wiki:
    • delete gpio lib manual install from wiki
    • change using usergroup-root override (now included in repo)
    • change using the reboot override
    • Raspberry Pi: add "dtparam=audio=on" to /boot/config.txt
      This enables on-board audio via Device Trees.
    • remove creating / setting alexapi home directory in PA section in the wiki
  • proper testing

Probably later:

  • non-interactive install - cli parameters or some sort of config file - this is important for testing / CI

@renekliment renekliment added this to the 1.3 milestone Nov 9, 2016
@renekliment renekliment self-assigned this Nov 9, 2016
@renekliment renekliment changed the title Setup script refactor [WIP] Setup script refactor Nov 9, 2016
@renekliment renekliment force-pushed the setup-script-basic branch 2 times, most recently from d9b35a7 to 167e001 Compare November 11, 2016 15:25
@renekliment renekliment changed the title [WIP] Setup script refactor Setup script refactor Nov 12, 2016
@renekliment
Copy link
Member Author

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 {
Copy link
Contributor

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

Copy link
Member Author

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!

@renekliment renekliment mentioned this pull request Nov 19, 2016
@maso27
Copy link
Contributor

maso27 commented Nov 21, 2016

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"
2.) follow the wiki to set up a "run as root" configuration
3.) set output device to "plughw:1" for my external sound card. (This one obviously depends on hardware, but is really the only practical way to use AlexaPi on CHIP right now.)

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?

@renekliment
Copy link
Member Author

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:

  1. set the default sound devices for CHIP ... and for Orange Pis too, because they usually have an internal mic
  2. set the platform in the config for CHIP and Orange Pi to dummy as default, since raspberrypi obviously won't work and this leaves a working, secure voice-activation only install
  3. include a function that's gonna tell the user that currently the only way for a button or LEDs to work on their device is the insecure root stuff and if they really wish to use it - if yes, then just activate the root override and set the platform in the config

Sounds okay?

@lechat
Copy link
Contributor

lechat commented Nov 21, 2016

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.

@renekliment
Copy link
Member Author

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 & systemctl daemon-reload) which can easily be found on the wiki. Currently the install is more of a one-way thing than a program which allows options. I'd like to get there in the future however. Also, the user would still have to set another platform in the config when disabling the override. I don't know, it might be just my mood. Hit me up with more reasons why we should do it now. Also I'd like it in the future if the setup script could do stuff like upgrading and stuff, where on/offing things would make also sense.

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.

@illperipherals
Copy link
Contributor

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?

@maso27
Copy link
Contributor

maso27 commented Nov 21, 2016

@renekliment
I'm perfectly fine with keeping root access as something that needs to be configured. I wanted to open the discussion, since it would be easy for a user to think they had done everything required to get AlexaPi to work on the CHIP, then to have it fail.

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.

@renekliment
Copy link
Member Author

@maso27 Don't get me wrong, you have fair points there. We're totally including the stuff in the setup script as mentioned above.

@renekliment
Copy link
Member Author

@illperipherals Feel free to open a PR. I don't have it on my todo / priority list.

@renekliment
Copy link
Member Author

Alright, so:

  1. The default audio devices should work better now.
  2. The default device_platform is dummy now, which works everywhere, so we don't have broken-by-default installs. Raspberrys are set-up correctly, because they work rootless.
  3. I wanted to set it up automatically even for root platforms, but that would take additional time with writing, testing and we would have no progress. So for OPis and CHIP, there is a message that tells users to follow the wiki if they want LEDs / button. We should do the automatic setting in the future though. I think it is important. I just feel like if I don't merge this soon, I won't have the strength to finish it later.

@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 config_set function, the root override is right there in the repo now, we can use sed for classic init script, etc. It's just a matter of using a few commands in the handle_root_platform function which already receives the platform name as it's argument. And a bit of testing of course.

@maso27
Copy link
Contributor

maso27 commented Nov 27, 2016

Works with CHIP!
I'd say we should merge it.

@renekliment
Copy link
Member Author

renekliment commented Nov 27, 2016

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!

@renekliment renekliment merged commit 6f64ae9 into alexa-pi:master Nov 27, 2016
@renekliment renekliment deleted the setup-script-basic branch November 27, 2016 19:46
@renekliment
Copy link
Member Author

Also, anyone feel free to take the automatic root platform setup, it's easy.

@renekliment renekliment mentioned this pull request Nov 27, 2016
5 tasks
@maso27
Copy link
Contributor

maso27 commented Nov 30, 2016

Huh. I guess I should've tested the Raspberry Pi again, instead of just focusing on the CHIP with #82. Sorry folks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants