-
Notifications
You must be signed in to change notification settings - Fork 110
fix(bluetooth): display notification when file transfer times out #1032
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
base: master
Are you sure you want to change the base?
Conversation
Fix the bug where timeout notification was not displayed during Bluetooth file transfer. The issue was introduced by dde-shell commit: linuxdeepin/dde-shell@c07b130 Changed the notifyID parameter to 0 in notifyReceiveFileTimeout to ensure the timeout notification is properly shown to the user. Log: display notification when file transfer times out Signed-off-by: ComixHe <heyuming@deepin.org>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnsures a Bluetooth file transfer timeout notification is shown by forcing the timeout notification call to use a notify ID of 0 instead of reusing the previous notification ID. Sequence diagram for Bluetooth file transfer timeout notificationsequenceDiagram
actor User
participant ObexAgent
participant NotificationService
User->>ObexAgent: StartBluetoothFileTransfer(deviceName, filename)
ObexAgent-->>User: TransferInProgress
ObexAgent-->>ObexAgent: DetectCancelReasonExpired
ObexAgent->>NotificationService: notifyReceiveFileTimeout(notify, 0, filename)
NotificationService-->>User: ShowTimeoutNotification(filename)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这段代码修改将 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
改进建议
总结:这个修改在语法上没有问题,但在业务逻辑和资源管理上存在潜在风险。建议在合并前,务必确认 |
There was a problem hiding this 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:
- Consider replacing the hard-coded
0notify ID with a named constant or helper to document why a zero value is required for the timeout notification to display correctly. - It may be helpful to add a short comment near this call (or in
notifyReceiveFileTimeout’s doc) explaining the behavioral difference whennotifyIDis 0 versus a non-zero value, so future changes don’t regress this behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider replacing the hard-coded `0` notify ID with a named constant or helper to document why a zero value is required for the timeout notification to display correctly.
- It may be helpful to add a short comment near this call (or in `notifyReceiveFileTimeout`’s doc) explaining the behavioral difference when `notifyID` is 0 versus a non-zero value, so future changes don’t regress this behavior.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, ComixHe The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Fix the bug where timeout notification was not displayed during Bluetooth file transfer. The issue was introduced by dde-shell commit:
linuxdeepin/dde-shell@c07b130
Changed the notifyID parameter to 0 in notifyReceiveFileTimeout to ensure the timeout notification is properly shown to the user.
Log: display notification when file transfer times out
Summary by Sourcery
Bug Fixes: