[PR #1359] [MERGED] Rework session keep-alive logic v2 #1359

New issue

Closed

opened 2026-02-27 20:02:09 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 20:02:09 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/librespot-org/librespot/pull/1359
Author: @wisp3rwind
Created: 10/2/2024
Status: ✅ Merged
Merged: 10/15/2024
Merged by: @roderickvd

Base: dev ← Head: pingpong-v2

📝 Commits (7)

8e99a69 session: Session::dispatch -> DispatchTask::dispatch
df6a09b session: DispatchTask is a regular struct instead of tuple struct
97a757d session: rework keepalive logic to match other clients
754c1b6 session: use pin projection instead of Boxing in DispatchTask
69f6153 update Cargo.lock
beff7e9 address review
38f8d37 increase keepalive timeouts

📊 Changes

3 files changed (+265 additions, -145 deletions)

View changed files

📝 Cargo.lock (+1 -0)
📝 core/Cargo.toml (+1 -0)
📝 core/src/session.rs (+263 -145)

📄 Description

Another approach to #1357 using the ideas from https://github.com/librespot-org/librespot/pull/1357#issuecomment-2386938448.

This looks like a big change, but much of it is just moving around code: fn dispatch is now a part of DispatchTask such that it has access to the fields of DispatchTask (namely, the timeout and state of the keepalive state machinery).

This is a little different from @roderickvd's idea: I've kept the KeepAliveState enum instead of just the ping_received flag, and instead there's only a single Sleep future around. The deadline of the latter is modified as required.

EDIT: Split into a few more commits now to make this easier to review. For testing, keep-alive events are logged at TRACE level.

Fixes #1340
Closes #1357

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/librespot-org/librespot/pull/1359 **Author:** [@wisp3rwind](https://github.com/wisp3rwind) **Created:** 10/2/2024 **Status:** ✅ Merged **Merged:** 10/15/2024 **Merged by:** [@roderickvd](https://github.com/roderickvd) **Base:** `dev` ← **Head:** `pingpong-v2` --- ### 📝 Commits (7) - [`8e99a69`](https://github.com/librespot-org/librespot/commit/8e99a69df0248ee6c69d7aa48345ad9f150cc151) session: Session::dispatch -> DispatchTask::dispatch - [`df6a09b`](https://github.com/librespot-org/librespot/commit/df6a09be4846cd618063ee0ef6f5132284734790) session: DispatchTask is a regular struct instead of tuple struct - [`97a757d`](https://github.com/librespot-org/librespot/commit/97a757dfff5512ed935a447f9f82419f4056fc2a) session: rework keepalive logic to match other clients - [`754c1b6`](https://github.com/librespot-org/librespot/commit/754c1b6c68f33586bba78b7e680a14f8c90bc474) session: use pin projection instead of Boxing in DispatchTask - [`69f6153`](https://github.com/librespot-org/librespot/commit/69f61532003489dc29dd4e4dd783c2fdb4e16fcb) update Cargo.lock - [`beff7e9`](https://github.com/librespot-org/librespot/commit/beff7e96a66dcba5c5e3439a367c763bcd3469ac) address review - [`38f8d37`](https://github.com/librespot-org/librespot/commit/38f8d3784bb22ea9da6b6426e119526a9ad4801a) increase keepalive timeouts ### 📊 Changes **3 files changed** (+265 additions, -145 deletions) <details> <summary>View changed files</summary> 📝 `Cargo.lock` (+1 -0) 📝 `core/Cargo.toml` (+1 -0) 📝 `core/src/session.rs` (+263 -145) </details> ### 📄 Description Another approach to #1357 using the ideas from https://github.com/librespot-org/librespot/pull/1357#issuecomment-2386938448. This looks like a big change, but much of it is just moving around code: `fn dispatch` is now a part of `DispatchTask` such that it has access to the fields of `DispatchTask` (namely, the timeout and state of the keepalive state machinery). This is a little different from @roderickvd's idea: I've kept the `KeepAliveState` enum instead of just the `ping_received` flag, and instead there's only a single `Sleep` future around. The deadline of the latter is modified as required. EDIT: Split into a few more commits now to make this easier to review. For testing, keep-alive events are logged at `TRACE` level. Fixes #1340 Closes #1357 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

kerem

2026-02-27 20:02:09 +03:00

closed this issue
added the
pull-request
label

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/librespot#1359

No description provided.

Rows
Columns