From 7b9e0bbaedf9559a1001d368dadd1260723d1406 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sat, 4 Nov 2023 14:10:29 -0700 Subject: [PATCH] Code cleanup, remove arm mac build (#216) * more code cleanup * remove mac aarch from build for now * gh actions * fix gh action --- .github/workflows/pull-request.yml | 34 +++++------------------------ .github/workflows/release.yml | 16 +++++++------- hifirs/src/cli.rs | 4 ++-- hifirs/src/cursive/mod.rs | 12 +++++----- hifirs/src/mpris.rs | 14 ++++++------ hifirs/src/player/controls.rs | 20 ++++++++++++----- hifirs/src/player/mod.rs | 32 +++++++++++++-------------- hifirs/src/player/queue/controls.rs | 4 ++-- hifirs/src/websocket.rs | 4 ++-- playlist-sync/src/cli.rs | 12 +++++----- playlist-sync/src/qobuz.rs | 8 +++---- qobuz-api/src/client/api.rs | 14 ++++++------ 12 files changed, 80 insertions(+), 94 deletions(-) diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index 2a8424d..50180f9 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -21,13 +21,11 @@ jobs: fail-fast: true matrix: os: [ubuntu-latest, macos-latest] - target: [x86_64-unknown-linux-gnu, x86_64-apple-darwin, aarch64-apple-darwin] + target: [x86_64-unknown-linux-gnu, x86_64-apple-darwin] rust: [stable] exclude: - os: ubuntu-latest target: x86_64-apple-darwin - - os: ubuntu-latest - target: aarch64-apple-darwin - os: macos-latest target: x86_64-unknown-linux-gnu env: @@ -48,7 +46,6 @@ jobs: run: | brew install gstreamer - name: Install Rust Toolchain - if: matrix.target != 'aarch64-unknown-linux-gnu' uses: actions-rs/toolchain@v1 with: profile: minimal @@ -57,15 +54,15 @@ jobs: components: rustfmt, clippy override: true - name: Install Cargo Plugins - if: matrix.target != 'aarch64-unknown-linux-gnu' - run: cargo install cargo-make cargo-insta sqlx-cli + uses: actions-rs/cargo@v1 + with: + command: install + args: cargo-make cargo-insta sqlx-cli - name: Create db file - if: matrix.target != 'aarch64-unknown-linux-gnu' run: "touch /tmp/data.db && cd hifirs && cargo sqlx database reset -y" env: DATABASE_URL: "sqlite:///tmp/data.db" - name: Run CI Flow - if: matrix.target != 'aarch64-unknown-linux-gnu' uses: actions-rs/cargo@v1 env: QOBUZ_USERNAME: ${{secrets.QOBUZ_USERNAME}} @@ -74,24 +71,3 @@ jobs: with: command: make args: ci-flow - # - name: Check arm64 - # uses: pguyot/arm-runner-action@v2 - # if: matrix.target == 'aarch64-unknown-linux-gnu' - # env: - # QOBUZ_USERNAME: ${{secrets.QOBUZ_USERNAME}} - # QOBUZ_PASSWORD: ${{secrets.QOBUZ_PASSWORD}} - # DATABASE_URL: "sqlite:///tmp/data.db" - # with: - # base_image: raspios_lite_arm64:latest - # import_github_env: true - # commands: | - # apt-get update -y --allow-releaseinfo-change - # apt-get install --no-install-recommends -y libunwind-dev libgstreamer1.0-dev - # curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh - # echo 'source $HOME/.cargo/env' >> $HOME/.bashrc - # export PATH="$HOME/.cargo/bin:${PATH}" - # rustup -y target add aarch64-unknown-linux-gnu - # rustup -y toolchain install stable-aarch64-unknown-linux-gnu - # cargo binstall --no-confirm cargo-make cargo-insta sqlx-cli - # touch /tmp/data.db && cargo sqlx database reset --source ./hifirs/migrations -y - # cargo make ci-flow diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index cab83b2..1ce41c8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -17,13 +17,11 @@ jobs: fail-fast: true matrix: os: [ubuntu-latest, macos-latest] - target: [x86_64-unknown-linux-gnu, x86_64-apple-darwin, aarch64-apple-darwin] + target: [x86_64-unknown-linux-gnu, x86_64-apple-darwin] rust: [stable] exclude: - os: ubuntu-latest target: x86_64-apple-darwin - - os: ubuntu-latest - target: aarch64-apple-darwin - os: macos-latest target: x86_64-unknown-linux-gnu env: @@ -52,25 +50,27 @@ jobs: - name: Install Rust Toolchain uses: actions-rs/toolchain@v1 with: - profile: minimal toolchain: stable target: ${{ matrix.target }} override: true + components: rust-src - name: Install Cargo Plugins - run: cargo install sqlx-cli + uses: actions-rs/install@v0.1 + with: + crate: sqlx-cli + version: latest + use-tool-cache: true - name: Create db file run: "touch /tmp/data.db && cd hifirs && cargo sqlx database reset -y" - env: + env: DATABASE_URL: "sqlite:///tmp/data.db" - name: Build ${{ matrix.target }} uses: actions-rs/cargo@v1 env: - PKG_CONFIG_ALLOW_CROSS: 1 SQLITE3_STATIC: true PKG_CONFIG_ALL_STATIC: true DATABASE_URL: "sqlite:///tmp/data.db" with: - use-cross: true command: build args: --bin hifi-rs --release --target=${{ matrix.target }} - name: Tar hifi-rs-${{ matrix.target }}.tar.gz diff --git a/hifirs/src/cli.rs b/hifirs/src/cli.rs index f2e60d6..7ee3ffe 100644 --- a/hifirs/src/cli.rs +++ b/hifirs/src/cli.rs @@ -273,7 +273,7 @@ pub async fn run() -> Result<(), Error> { ) .await?; - player::play_uri(url).await?; + player::play_uri(&url).await?; wait!(mut handles, cli.disable_tui); @@ -307,7 +307,7 @@ pub async fn run() -> Result<(), Error> { ) .await?; - player::play_album(album_id).await?; + player::play_album(&album_id).await?; wait!(mut handles, cli.disable_tui); diff --git a/hifirs/src/cursive/mod.rs b/hifirs/src/cursive/mod.rs index 94bcdc0..060238f 100644 --- a/hifirs/src/cursive/mod.rs +++ b/hifirs/src/cursive/mod.rs @@ -374,7 +374,7 @@ impl CursiveUI { let open = Arc::new(move |s: &mut Cursive| { let mut panel = CursiveUI::enter_url(move |s, url| { let u = url.to_string(); - tokio::spawn(async move { CONTROLS.play_uri(u).await }); + tokio::spawn(async move { CONTROLS.play_uri(&u).await }); s.pop_layer(); ENTER_URL_OPEN.store(false, Ordering::Relaxed); }); @@ -549,8 +549,8 @@ fn load_search_results(item: &str, s: &mut Cursive) { search_results.set_on_submit(move |_s: &mut Cursive, item: &String| { if item != UNSTREAMABLE { - let i = item.clone(); - tokio::spawn(async move { CONTROLS.play_album(i).await }); + let item = item.clone(); + tokio::spawn(async move { CONTROLS.play_album(&item).await }); } }); } @@ -671,7 +671,7 @@ fn submit_artist(s: &mut Cursive, item: i32) { tree.add_leaf(a.list_item(), move |s: &mut Cursive| { let id = a.id.clone(); - tokio::spawn(async move { CONTROLS.play_album(id).await }); + tokio::spawn(async move { CONTROLS.play_album(&id).await }); s.call_on_name( "screens", @@ -726,8 +726,8 @@ fn submit_track(s: &mut Cursive, item: (i32, Option)) { s.screen_mut().pop_layer(); if let Some(album_id) = &item.1 { - let a = album_id.clone(); - tokio::spawn(async move { CONTROLS.play_album(a).await }); + let id = album_id.clone(); + tokio::spawn(async move { CONTROLS.play_album(&id).await }); s.call_on_name( "screens", diff --git a/hifirs/src/mpris.rs b/hifirs/src/mpris.rs index 2ebea11..23c5487 100644 --- a/hifirs/src/mpris.rs +++ b/hifirs/src/mpris.rs @@ -258,7 +258,7 @@ pub struct MprisPlayer { #[dbus_interface(name = "org.mpris.MediaPlayer2.Player")] impl MprisPlayer { - async fn open_uri(&self, uri: String) { + async fn open_uri(&self, uri: &str) { self.controls.play_uri(uri).await; } async fn play(&self) { @@ -280,13 +280,13 @@ impl MprisPlayer { self.controls.previous().await; } #[dbus_interface(property, name = "PlaybackStatus")] - async fn playback_status(&self) -> String { + async fn playback_status(&self) -> &str { match self.status { - GstState::Playing => "Playing".to_string(), - GstState::Paused => "Paused".to_string(), - GstState::Null => "Stopped".to_string(), - GstState::VoidPending => "Stopped".to_string(), - GstState::Ready => "Ready".to_string(), + GstState::Playing => "Playing", + GstState::Paused => "Paused", + GstState::Null => "Stopped", + GstState::VoidPending => "Stopped", + GstState::Ready => "Ready", } } #[dbus_interface(property, name = "LoopStatus")] diff --git a/hifirs/src/player/controls.rs b/hifirs/src/player/controls.rs index 9fd6abb..2bb6c41 100644 --- a/hifirs/src/player/controls.rs +++ b/hifirs/src/player/controls.rs @@ -75,11 +75,21 @@ impl Controls { pub async fn jump_backward(&self) { action!(self, Action::JumpBackward); } - pub async fn play_album(&self, album_id: String) { - action!(self, Action::PlayAlbum { album_id }); - } - pub async fn play_uri(&self, uri: String) { - action!(self, Action::PlayUri { uri }); + pub async fn play_album(&self, album_id: &str) { + action!( + self, + Action::PlayAlbum { + album_id: album_id.to_string() + } + ); + } + pub async fn play_uri(&self, uri: &str) { + action!( + self, + Action::PlayUri { + uri: uri.to_string() + } + ); } pub async fn play_track(&self, track_id: i32) { action!(self, Action::PlayTrack { track_id }); diff --git a/hifirs/src/player/mod.rs b/hifirs/src/player/mod.rs index 9bd321b..cc167ca 100644 --- a/hifirs/src/player/mod.rs +++ b/hifirs/src/player/mod.rs @@ -205,10 +205,10 @@ pub async fn set_player_state(state: gst::State) -> Result<()> { Ok(()) } -async fn broadcast_track_list<'a>(list: TrackListValue) -> Result<()> { +async fn broadcast_track_list<'a>(list: &TrackListValue) -> Result<()> { BROADCAST_CHANNELS .tx - .broadcast(Notification::CurrentTrackList { list }) + .broadcast(Notification::CurrentTrackList { list: list.clone() }) .await?; Ok(()) } @@ -390,7 +390,7 @@ pub async fn skip(new_position: u32) -> Result<()> { drop(state); - broadcast_track_list(list).await?; + broadcast_track_list(&list).await?; BROADCAST_CHANNELS .tx .broadcast(Notification::Position { @@ -415,7 +415,7 @@ pub async fn play_track(track_id: i32) -> Result<()> { if let Some(track_url) = state.play_track(track_id).await { let list = state.track_list(); - broadcast_track_list(list).await?; + broadcast_track_list(&list).await?; drop(state); @@ -428,14 +428,14 @@ pub async fn play_track(track_id: i32) -> Result<()> { } #[instrument] /// Plays a full album. -pub async fn play_album(album_id: String) -> Result<()> { +pub async fn play_album(album_id: &str) -> Result<()> { ready().await?; let mut state = QUEUE.get().unwrap().write().await; if let Some(track_url) = state.play_album(album_id).await { let list = state.track_list(); - broadcast_track_list(list).await?; + broadcast_track_list(&list).await?; drop(state); @@ -454,7 +454,7 @@ pub async fn play_playlist(playlist_id: i64) -> Result<()> { let mut state = QUEUE.get().unwrap().write().await; if let Some(track_url) = state.play_playlist(playlist_id).await { let list = state.track_list(); - broadcast_track_list(list).await?; + broadcast_track_list(&list).await?; drop(state); @@ -467,11 +467,11 @@ pub async fn play_playlist(playlist_id: i64) -> Result<()> { } #[instrument] /// Play an item from Qobuz web uri -pub async fn play_uri(uri: String) -> Result<()> { - match client::parse_url(uri.as_str()) { +pub async fn play_uri(uri: &str) -> Result<()> { + match client::parse_url(uri) { Ok(url) => match url { UrlType::Album { id } => { - play_album(id).await?; + play_album(&id).await?; } UrlType::Playlist { id } => { play_playlist(id).await?; @@ -687,12 +687,12 @@ pub async fn player_loop() -> Result<()> { } Some(msg) = messages.next() => { if msg.type_() == MessageType::Buffering { - match handle_message(msg).await { + match handle_message(&msg).await { Ok(_) => {}, Err(error) => debug!(?error), }; } else { - tokio::spawn(async { match handle_message(msg).await { + tokio::spawn(async move { match handle_message(&msg).await { Ok(()) => {} Err(error) => {debug!(?error);} } @@ -728,13 +728,13 @@ async fn handle_action(action: Action) -> Result<()> { } Action::Stop => stop().await?, Action::PlayAlbum { album_id } => { - play_album(album_id).await?; + play_album(&album_id).await?; } Action::PlayTrack { track_id } => { play_track(track_id).await?; } Action::PlayUri { uri } => { - play_uri(uri).await?; + play_uri(&uri).await?; } Action::PlayPlaylist { playlist_id } => { play_playlist(playlist_id).await?; @@ -754,7 +754,7 @@ async fn handle_action(action: Action) -> Result<()> { Ok(()) } -async fn handle_message(msg: Message) -> Result<()> { +async fn handle_message(msg: &Message) -> Result<()> { match msg.view() { MessageView::Eos(_) => { debug!("END OF STREAM"); @@ -771,7 +771,7 @@ async fn handle_message(msg: Message) -> Result<()> { MessageView::StreamStart(_) => { if is_playing() { let list = QUEUE.get().unwrap().read().await.track_list(); - broadcast_track_list(list).await?; + broadcast_track_list(&list).await?; } } MessageView::AsyncDone(msg) => { diff --git a/hifirs/src/player/queue/controls.rs b/hifirs/src/player/queue/controls.rs index b2aebe2..7f5676c 100644 --- a/hifirs/src/player/queue/controls.rs +++ b/hifirs/src/player/queue/controls.rs @@ -69,10 +69,10 @@ impl From for SavedState { } impl PlayerState { - pub async fn play_album(&mut self, album_id: String) -> Option { + pub async fn play_album(&mut self, album_id: &str) -> Option { debug!("setting up album to play"); - if let Some(album) = self.service.album(album_id.as_str()).await { + if let Some(album) = self.service.album(album_id).await { let mut tracklist = TrackListValue::new(Some(&album.tracks)); tracklist.set_album(album); tracklist.set_list_type(TrackListType::Album); diff --git a/hifirs/src/websocket.rs b/hifirs/src/websocket.rs index 2a603cd..a9d4a6e 100644 --- a/hifirs/src/websocket.rs +++ b/hifirs/src/websocket.rs @@ -165,12 +165,12 @@ async fn handle_connection(socket: WebSocket) { Action::JumpForward => controls.jump_forward().await, Action::JumpBackward => controls.jump_backward().await, Action::PlayAlbum { album_id } => { - controls.play_album(album_id).await + controls.play_album(&album_id).await } Action::PlayTrack { track_id } => { controls.play_track(track_id).await } - Action::PlayUri { uri } => controls.play_uri(uri).await, + Action::PlayUri { uri } => controls.play_uri(&uri).await, Action::PlayPlaylist { playlist_id } => { controls.play_playlist(playlist_id).await } diff --git a/playlist-sync/src/cli.rs b/playlist-sync/src/cli.rs index 4899097..8560588 100644 --- a/playlist-sync/src/cli.rs +++ b/playlist-sync/src/cli.rs @@ -114,13 +114,13 @@ pub async fn run() -> Result<(), Error> { if !results.is_empty() { if let Some(found) = results.get(0) { qobuz - .add_track(qobuz_playlist.id(), found.id.to_string()) + .add_track(&qobuz_playlist.id(), &found.id.to_string()) .await; qobuz .update_track_position( - qobuz_playlist.id(), - found.id.to_string(), + &qobuz_playlist.id(), + &found.id.to_string(), track_position, ) .await?; @@ -151,14 +151,14 @@ pub async fn run() -> Result<(), Error> { if !results.is_empty() { if let Some(found) = results.get(0) { qobuz - .add_track(qobuz_playlist.id(), found.id.to_string()) + .add_track(&qobuz_playlist.id(), &found.id.to_string()) .await; if missing.index < qobuz_playlist.track_count() { qobuz .update_track_position( - qobuz_playlist.id(), - found.id.to_string(), + &qobuz_playlist.id(), + &found.id.to_string(), missing.index - 1, ) .await?; diff --git a/playlist-sync/src/qobuz.rs b/playlist-sync/src/qobuz.rs index 363563a..7ebc915 100644 --- a/playlist-sync/src/qobuz.rs +++ b/playlist-sync/src/qobuz.rs @@ -58,12 +58,12 @@ impl<'q> Qobuz<'q> { results.tracks.items } - pub async fn add_track(&self, playlist_id: String, track_id: String) { + pub async fn add_track(&self, playlist_id: &str, track_id: &str) { self.progress .set_message(format!("adding {track_id} to {playlist_id}")); match self .client - .playlist_add_track(playlist_id.clone(), vec![track_id.clone()]) + .playlist_add_track(playlist_id, vec![track_id]) .await { Ok(_) => debug!("track added"), @@ -75,8 +75,8 @@ impl<'q> Qobuz<'q> { pub async fn update_track_position( &self, - playlist_id: String, - track_id: String, + playlist_id: &str, + track_id: &str, index: usize, ) -> hifirs_qobuz_api::Result<()> { self.client diff --git a/qobuz-api/src/client/api.rs b/qobuz-api/src/client/api.rs index 887e060..3490322 100644 --- a/qobuz-api/src/client/api.rs +++ b/qobuz-api/src/client/api.rs @@ -319,15 +319,15 @@ impl Client { /// Add new track to playlist pub async fn playlist_add_track( &self, - playlist_id: String, - track_ids: Vec, + playlist_id: &str, + track_ids: Vec<&str>, ) -> Result { let endpoint = format!("{}{}", self.base_url, Endpoint::PlaylistAddTracks); let track_ids = track_ids.join(","); let mut form_data = HashMap::new(); - form_data.insert("playlist_id", playlist_id.as_str()); + form_data.insert("playlist_id", playlist_id); form_data.insert("track_ids", track_ids.as_str()); form_data.insert("no_duplicate", "true"); @@ -355,16 +355,16 @@ impl Client { pub async fn playlist_track_position( &self, index: usize, - playlist_id: String, - track_id: String, + playlist_id: &str, + track_id: &str, ) -> Result { let endpoint = format!("{}{}", self.base_url, Endpoint::PlaylistUpdatePosition); let index = index.to_string(); let mut form_data = HashMap::new(); - form_data.insert("playlist_id", playlist_id.as_str()); - form_data.insert("playlist_track_ids", track_id.as_str()); + form_data.insert("playlist_id", playlist_id); + form_data.insert("playlist_track_ids", track_id); form_data.insert("insert_before", index.as_str()); post!(self, &endpoint, form_data)