Skip to content
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

Add default streams for first startup #1023

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

SteveMicroNova
Copy link
Contributor

@SteveMicroNova SteveMicroNova commented Jan 29, 2025

Sets system default state to Groove Salad connected to all zones at 0 volume and muted everywhere, as well as Spotify and Airplay2 sources connected to no zones for first startup
Can be tested by going to Setttings -> Config -> Factory Reset

Checklist

  • Have you tested your changes and ensured they work?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Does your submission pass linting & tests? You can test on localhost using ./scripts/test

@linknum23
Copy link
Contributor

Wasn't the plan to also have 2 Spotify and 2 Airplay streams added out of the box. Additionally there would be one of each of those that was connected to a source but not connected to any zones.

@linknum23
Copy link
Contributor

I have this debate as to whether it makes sense to start with some small volume instead of being completely muted.

@SteveMicroNova
Copy link
Contributor Author

Wasn't the plan to also have 2 Spotify and 2 Airplay streams added out of the box. Additionally there would be one of each of those that was connected to a source but not connected to any zones.

I feel that we've gone back and forth on this, so I did the smallest lift. I can happily go back to the airplay+spotify version of this intent

I have this debate as to whether it makes sense to start with some small volume instead of being completely muted.

I'd argue completely muted, I don't want to be responsible for figuring out what is a good volume to have it audible on quiet speakers but not explode loud ones before they can dial in their DB gate settings.

@SteveMicroNova SteveMicroNova changed the title Add Groove Salad as a default station on first startup Add default streams for first startup Jan 29, 2025
Comment on lines -22 to -25
{"id": 0, "name": "Input 1", "input": ""},
{"id": 1, "name": "Input 2", "input": ""},
{"id": 2, "name": "Input 3", "input": ""},
{"id": 3, "name": "Input 4", "input": ""},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that these being called inputs is used anywhere, but they are outputs so I made sure they were named that in line with the streamer section as well
Correct me if needed

@linknum23
Copy link
Contributor

We could potentially setup 3 sources in the cleanup script. One connecting groove salad to the first source with all 6 zones, with the 2 other sources connected to a spotify and an airplay stream respectively.

@SteveMicroNova
Copy link
Contributor Author

I've updated the cleanup script and ran it against my dev unit, this is the result:
image

@linknum23
Copy link
Contributor

Looks good. Any reason these streams shouldn't each be named AmpliPi 1 and AmpliPi 2? Or maybe AmpliPro 1 and AmpliPro 2? I would expect them to be called be the name of the device initially when trying to find them on Spotify or Airplay. Most people will have several spotify/airplay capable devices at home and this will make them easy to identify initially.
@gorski123 any thoughts are what name they should show up as?

@SteveMicroNova
Copy link
Contributor Author

Looks good. Any reason these streams shouldn't each be named AmpliPi 1 and AmpliPi 2? Or maybe AmpliPro 1 and AmpliPro 2?

We add 4 new streams in this PR: SpotiPi 1, SpotiPi 2, AmpliPlay 1, and AmpliPlay 2. The ones that connect by default are SpotiPi 1 (because that's the first one) and AmpliPlay 2 (because there is one that is Airplay 1 and one that is Airplay 2, and having those numbers not match up seemed bothersome)
They're named what they are because I wanted to cleverly say "This is this stream time on your AmpliPi" with only one word to ensure the name would fit on the devices you're connecting from

@linknum23
Copy link
Contributor

Yeah I get that. I guess I just have a preference for the Spotify Streams to be name AmpliPi 1, AmpliPi 2 and also have the Airplay streams be name AmpliPi 1 and AmpliPi 2. I realize the names clash but I fee like that may give the user a less clunky experience. I realize this is an opinion which is why I was asking for input from Jason. I should probably as @klay2000 as well.

@klay2000
Copy link
Contributor

klay2000 commented Feb 6, 2025

Airplay streams show up in Spotify too on IOS right? If I'm remembering that correctly you should def not have any overlap.

I actually don't mind the funny names but APSpotify/APAirplay would be more professional.

@SteveMicroNova
Copy link
Contributor Author

Airplay streams show up in Spotify too on IOS right? If I'm remembering that correctly you should def not have any overlap.

image

No, Airplay does show up but you have to open it as a separate window so that should be safe

@klay2000
Copy link
Contributor

klay2000 commented Feb 6, 2025

Oh ok, call it Amplipro then?

…rst startup

Update cleanup script to connect default streams
@SteveMicroNova
Copy link
Contributor Author

image

@SteveMicroNova
Copy link
Contributor Author

Jason has also said "AmpliPro 1 and 2 is fine for all of those" so that seems to be the way forward
I've made this change, so now that we have consensus this is due to be approved I'd think

Comment on lines +37 to +40
{"id": 1001, "name": "AmpliPi 1", "type": "spotify", "disabled": False, "browsable": False},
{"id": 1002, "name": "AmpliPi 2", "type": "spotify", "disabled": False, "browsable": False},
{"id": 1003, "name": "AmpliPi 1", "type": "airplay", "disabled": False, "ap2": False, "browsable": False},
{"id": 1004, "name": "AmpliPi 2", "type": "airplay", "disabled": False, "ap2": True, "browsable": False}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should probably be called AmpliPro 1/2 as well right?

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

Successfully merging this pull request may close these issues.

3 participants