Skip to content

Comments

fix: fix sound effect process exit and optimize bluetooth notifications#1024

Merged
fly602 merged 1 commit intolinuxdeepin:masterfrom
fly602:fix-soundEffect
Feb 4, 2026
Merged

fix: fix sound effect process exit and optimize bluetooth notifications#1024
fly602 merged 1 commit intolinuxdeepin:masterfrom
fly602:fix-soundEffect

Conversation

@fly602
Copy link
Contributor

@fly602 fly602 commented Feb 4, 2026

Fixed sound effect service not exiting properly during logout by adding systemd service configuration. Enhanced Bluetooth file transfer notifications to properly close previous notifications and use new notification IDs.

The changes include:

  1. Added SystemdService directive to D-Bus service file for proper service management
  2. Created systemd user service unit file for sound effect service with proper session integration
  3. Modified Bluetooth OBEX agent to close previous notifications before sending new ones
  4. Changed notification ID handling to use 0 (new notification) instead of replaceID for better user experience

Log: Improved Bluetooth file transfer notification behavior and fixed sound effect service lifecycle

Influence:

  1. Test Bluetooth file transfer notifications - they should properly replace old notifications
  2. Verify sound effect service exits correctly during logout/reboot
  3. Check that new Bluetooth transfer notifications appear correctly
  4. Test system stability when multiple file transfers occur simultaneously
  5. Verify sound effects work normally during regular usage

fix: 修复音效进程退出问题并优化蓝牙通知

修复注销时音效进程未正确退出的问题,通过添加 systemd 服务配置。优化蓝牙
文件传输通知,正确关闭先前通知并使用新的通知ID。

具体修改包括:

  1. 在 D-Bus 服务文件中添加 SystemdService 指令以实现正确的服务管理
  2. 为音效服务创建 systemd 用户服务单元文件,实现正确的会话集成
  3. 修改蓝牙 OBEX 代理,在发送新通知前正确关闭先前通知
  4. 更改通知 ID 处理,使用 0(新通知)而非 replaceID 以提升用户体验

Log: 改进蓝牙文件传输通知行为并修复音效服务生命周期问题
PMS: BUG-334279 BUG-299605
Influence:

  1. 测试蓝牙文件传输通知 - 应正确替换旧通知
  2. 验证音效服务在注销/重启时正确退出
  3. 检查新的蓝牙传输通知是否正确显示
  4. 测试多个文件同时传输时的系统稳定性
  5. 验证正常使用期间音效功能正常工作

Summary by Sourcery

Improve lifecycle management of the sound effect service and refine Bluetooth file transfer notification behavior.

Bug Fixes:

  • Ensure the sound effect D-Bus service is managed by a systemd user unit so the process exits correctly on logout or reboot.
  • Fix Bluetooth file transfer notifications so previous notifications are explicitly closed before new ones are shown, preventing stale or overlapping notifications.

Enhancements:

  • Change Bluetooth file transfer notifications to always create new notification entries instead of reusing IDs, improving clarity of transfer status for users.

@sourcery-ai
Copy link

sourcery-ai bot commented Feb 4, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Implements proper systemd-based lifecycle management for the sound effect D-Bus service and refines Bluetooth OBEX file-transfer notifications to close previous notifications and always create fresh ones instead of reusing IDs.

Sequence diagram for updated Bluetooth transfer notification behavior

sequenceDiagram
    actor User
    participant Device as RemoteDevice
    participant ObexAgent as ObexAgent
    participant Notify as NotificationsService

    User->>Device: Initiate Bluetooth file transfer
    Device->>ObexAgent: OBEX transfer events

    rect rgb(235, 245, 255)
        Note over ObexAgent: On transfer progress/completion
        ObexAgent->>Notify: CloseNotification(0, oldNotifyID)
        ObexAgent->>Notify: Notify(appID=0, appName=dde-control-center, replacesID=0, icon=notifyIconBluetoothConnected, summary, body, actions, hints)
        Notify-->>ObexAgent: newNotifyID
    end

    rect rgb(255, 235, 235)
        Note over ObexAgent: On transfer failure
        ObexAgent->>Notify: CloseNotification(0, oldNotifyID)
        ObexAgent->>Notify: Notify(appID=0, appName=dde-control-center, replacesID=0, icon=notifyIconBluetoothConnectFailed, summary, body, actions, hints)
        Notify-->>ObexAgent: failureNotifyID
    end

    Notify-->>User: Fresh notification shown for each result
Loading

State diagram for SoundEffect1 service lifecycle with systemd management

stateDiagram-v2
    [*] --> Inactive

    Inactive --> Activating: D-Bus Name Requested
    Activating --> Active: systemd starts SoundEffect1 service

    Active --> Stopping: Session logout or reboot
    Stopping --> Inactive: systemd stops SoundEffect1 service

    Active --> Inactive: No clients and idle timeout (if configured)

    Inactive --> [*]
Loading

File-Level Changes

Change Details Files
Ensure Bluetooth OBEX transfer notifications close previous notifications and use new IDs instead of reusing replace IDs.
  • In notifyProgress, call CloseNotification with the previous notification ID before sending a new notification.
  • In notifyProgress, change the Notify call to use 0 as the notification ID to force creation of a new notification.
  • In notifyFailed, call CloseNotification with the previous notification ID before sending the failure notification.
  • In notifyFailed, change the Notify call to use 0 as the notification ID to always create a new failure notification.
bluetooth1/obex_agent.go
Move the sound effect D-Bus service under explicit systemd user service management to ensure it exits correctly on logout/reboot.
  • Add the SystemdService directive to the D-Bus service file so the sound effect service is managed by systemd rather than spawned directly by D-Bus.
  • Introduce a systemd user unit org.deepin.dde.SoundEffect1.service to define how the sound effect service runs and integrates with the user session.
misc/services/org.deepin.dde.SoundEffect1.service
misc/systemd/services/user/org.deepin.dde.SoundEffect1.service

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • In both notifyProgress and notifyFailed, consider guarding CloseNotification with a replaceID != 0 check and handling/logging any error it returns so you don't make a D-Bus call for the default zero ID or silently ignore failures.
  • The change from using replaceID in Notify to always passing 0 will cause every transfer to create a new notification; if the intent is to avoid duplicates while still updating existing toasts, you may want to revisit whether a per-transfer ID or tag can be used instead of completely disabling replacement.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In both `notifyProgress` and `notifyFailed`, consider guarding `CloseNotification` with a `replaceID != 0` check and handling/logging any error it returns so you don't make a D-Bus call for the default zero ID or silently ignore failures.
- The change from using `replaceID` in `Notify` to always passing `0` will cause every transfer to create a new notification; if the intent is to avoid duplicates while still updating existing toasts, you may want to revisit whether a per-transfer ID or tag can be used instead of completely disabling replacement.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Fixed sound effect service not exiting properly during logout by
adding systemd service configuration. Enhanced Bluetooth file transfer
notifications to properly close previous notifications and use new
notification IDs.

The changes include:
1. Added SystemdService directive to D-Bus service file for proper
service management
2. Created systemd user service unit file for sound effect service with
proper session integration
3. Modified Bluetooth OBEX agent to close previous notifications before
sending new ones
4. Changed notification ID handling to use 0 (new notification) instead
of replaceID for better user experience

Log: Improved Bluetooth file transfer notification behavior and fixed
sound effect service lifecycle

Influence:
1. Test Bluetooth file transfer notifications - they should properly
replace old notifications
2. Verify sound effect service exits correctly during logout/reboot
3. Check that new Bluetooth transfer notifications appear correctly
4. Test system stability when multiple file transfers occur
simultaneously
5. Verify sound effects work normally during regular usage

fix: 修复音效进程退出问题并优化蓝牙通知

修复注销时音效进程未正确退出的问题,通过添加 systemd 服务配置。优化蓝牙
文件传输通知,正确关闭先前通知并使用新的通知ID。

具体修改包括:
1. 在 D-Bus 服务文件中添加 SystemdService 指令以实现正确的服务管理
2. 为音效服务创建 systemd 用户服务单元文件,实现正确的会话集成
3. 修改蓝牙 OBEX 代理,在发送新通知前正确关闭先前通知
4. 更改通知 ID 处理,使用 0(新通知)而非 replaceID 以提升用户体验

Log: 改进蓝牙文件传输通知行为并修复音效服务生命周期问题
PMS: BUG-334279 BUG-299605
Influence:
1. 测试蓝牙文件传输通知 - 应正确替换旧通知
2. 验证音效服务在注销/重启时正确退出
3. 检查新的蓝牙传输通知是否正确显示
4. 测试多个文件同时传输时的系统稳定性
5. 验证正常使用期间音效功能正常工作
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码 diff 包含了蓝牙传输通知逻辑的修改和 SoundEffect 服务的 systemd 集成方式的变更。以下是对这两部分内容的详细审查和改进建议:

1. 蓝牙通知逻辑修改 (bluetooth1/obex_agent.go)

变更分析:
notifyProgressnotifyFailed 两个函数中,代码变更主要涉及通知的替换逻辑。

  • 变更前:调用 notify.Notify 时传入 replaceID 作为 replacesID 参数,意图是覆盖旧的通知 ID。
  • 变更后:在调用 notify.Notify 之前,显式调用了 notify.CloseNotification(0, replaceID) 关闭旧通知,并且在 notify.Notify 中将 replacesID 参数设为 0

审查意见:

  • 逻辑正确性:修改后的逻辑在功能上是可行的。先关闭旧通知再发送新通知,确实能实现"更新"通知的效果。将 replacesID 设为 0 意味着新生成的通知将获得一个新的 ID,而不是继承旧 ID。
  • 代码质量与冗余:这种做法略显冗余。D-Bus 通知规范(以及 notify.Notify 函数的设计)本身就支持通过传入 replacesID 参数来更新现有通知,无需先手动关闭。手动关闭再新建可能会导致通知栏出现闪烁(旧通知消失,新通知出现),而直接替换则更加平滑。
  • 潜在风险:如果 replaceID 无效(例如通知已被用户关闭或超时),CloseNotification 可能会静默失败或抛出错误(取决于具体实现),虽然代码中未处理其返回值,但这通常不会导致严重后果。

改进建议:
建议回退这部分修改,保持原有的使用 replaceID 进行替换的逻辑,除非底层通知库存在特定的 Bug 导致必须手动关闭。如果必须手动关闭,建议检查 replaceID 是否为 0(无效 ID),避免无意义的调用。

// 建议的改进写法(如果底层库支持标准替换行为):
// 恢复原来的逻辑,去掉 CloseNotification 调用,并将 replaceID 传回 Notify 函数
// notifyID, err = notify.Notify(0, ..., replaceID, ...)

2. SoundEffect 服务配置修改

变更分析:
这部分变更将 org.deepin.dde.SoundEffect1 服务的启动方式从传统的 D-Bus 激活(通过 .service 文件直接指定 Exec)改为 Systemd 激活(通过 D-Bus 指向 SystemdService,并由 systemd 单元文件管理)。

  • org.deepin.dde.SoundEffect1.service (D-BUS Service):
    • Exec=/bin/false: 这是一个常见的技巧,用于防止 D-Bus 守护进程直接尝试启动服务,将启动权完全移交给 systemd。
    • SystemdService=...: 声明由名为 org.deepin.dde.SoundEffect1.service 的 systemd 单元文件负责管理该服务。
  • org.deepin.dde.SoundEffect1.service (Systemd User Service):
    • 新增了 systemd 用户服务单元文件。
    • Type=dbus: 告诉 systemd 该服务是一个 D-Bus 服务,当服务在总线上注册了指定的 BusName 后,才视为启动成功。
    • BusName=...: 指定要注册的 D-Bus 名称。
    • PartOf=graphical-session.target: 确保服务随图形会话一起管理和生命周期绑定。
    • Restart=on-failure: 设置了自动重启策略。

审查意见:

  • 架构与性能:这是一个非常好的改进。使用 systemd 管理 D-Bus 服务是现代 Linux 发行版(特别是使用 systemd 的系统)的最佳实践。它带来了以下好处:
    • 更好的依赖管理和生命周期控制(通过 PartOf, After 等)。
    • 统一的日志管理(journalctl)。
    • 更可靠的崩溃重启机制(Restart=on-failure)。
    • 进程状态监控。
  • 安全性:配置中未看到 User= 或动态用户(DynamicUser=)的设置。如果这是一个以 root 权限运行的系统服务,建议检查是否可以降权运行。不过考虑到它是用户服务(路径在 .../user/...),它通常继承用户会话的权限,这是合理的。
  • 语法逻辑:Systemd 单元文件的语法是正确的。
    • RefuseManualStart=noRefuseManualStop=no 是默认值,可以省略,但写出来也无妨,增加可读性。
    • CollectMode=inactive-or-failed 是较新的 systemd 版本特性,用于清理垃圾回收,有助于系统资源管理,但在旧版 systemd 上可能会导致错误或被忽略。需确认目标系统的 systemd 版本。

改进建议:

  1. Systemd 配置微调

    • 可以考虑在 [Unit] 部分添加 Documentation=man:soundeffect(1)(如果有文档),增加规范性。
    • 如果服务对时间敏感或需要快速响应,可以调整 RestartSec,目前 1s 是合理的。
  2. 兼容性检查

    • 确认目标环境是否支持 CollectMode(systemd >= 244)。如果需要兼容旧版本,建议移除该行。

总结

  1. Obex Agent 部分:建议撤销修改。原来的 replaceID 替换逻辑更符合 D-Bus 通知规范,用户体验更好(无闪烁)。除非有特定的 Bug 需要通过 CloseNotification 规避。
  2. SoundEffect 部分:建议采纳。这是从 D-Bus 激活迁移到 Systemd 激活的标准做法,提升了服务的稳定性和可维护性。只需注意 CollectMode 的兼容性问题。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602, mhduiy

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fly602 fly602 merged commit 110fc6f into linuxdeepin:master Feb 4, 2026
17 checks passed
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