-
Notifications
You must be signed in to change notification settings - Fork 102
fix: Fix the issue of the storage message click having no effect #918
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
Conversation
Reviewer's GuideThis PR refactors the OBEX agent’s notification handling by replacing the boolean channel with a string-based one to distinguish user actions and cancel events, extends the notification timeout, and updates the requestReceive logic to handle clicks and cancels correctly. Sequence diagram for updated requestReceive notification handlingsequenceDiagram
participant "User"
participant "obexAgent"
participant "notify"
User->>notify: Clicks notification ("receive" or "decline")
notify->>obexAgent: ConnectActionInvoked(actionKey)
obexAgent->>obexAgent: requestNotifyCh <- "receive" or "decline"
obexAgent->>obexAgent: select on requestNotifyCh
alt actionKey == "receive"
obexAgent->>obexAgent: result = true
else actionKey == "decline"
obexAgent->>obexAgent: result = false
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review我对这段代码进行了仔细审查,以下是我的分析和改进建议: 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
具体改进建议代码:// 定义常量提高可维护性
const (
ActionObexCancel = "obex_cancel"
ActionDecline = "decline"
ActionReceive = "receive"
)
func (a *obexAgent) Cancel() *dbus.Error {
a.requestNotifyChMu.Lock()
defer a.requestNotifyChMu.Unlock()
if a.requestNotifyCh == nil {
return nil
}
// 添加超时保护
select {
case a.requestNotifyCh <- ActionObexCancel:
case <-time.After(100 * time.Millisecond):
logger.Warning("Failed to send cancel notification, channel might be blocked")
}
return nil
}
func (a *obexAgent) requestReceive(deviceName, filename string) (bool, error) {
// ... 其他代码 ...
a.requestNotifyChMu.Lock()
// 使用对象池或重用通道
if a.requestNotifyCh == nil {
a.requestNotifyCh = make(chan string, 10)
} else {
// 清空通道中的旧数据
for len(a.requestNotifyCh) > 0 {
<-a.requestNotifyCh
}
}
a.requestNotifyChMu.Unlock()
// ... 其他代码 ...
var result bool
select {
case reason := <-a.requestNotifyCh:
switch reason {
case ActionObexCancel:
// cancel from obexd, treat as timeout notify
a.b.setPropTransportable(true)
a.notifyReceiveFileTimeout(notify, notifyID, filename)
result = false
case ActionDecline:
// user declined, do not send timeout notify
a.b.setPropTransportable(true)
result = false
case ActionReceive:
result = true
default:
logger.Warningf("Unknown action received: %s", reason)
result = false
}
case <-time.After(receiveFileTimeout):
a.b.setPropTransportable(true)
// ... 超时处理 ...
}
// 清理资源
a.requestNotifyChMu.Lock()
close(a.requestNotifyCh)
a.requestNotifyCh = nil
a.requestNotifyChMu.Unlock()
return result, nil
}这些改进提高了代码的可维护性、安全性和性能,同时保持了原有的功能完整性。 |
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.
|
TAG Bot New tag: 6.1.57 |
|
TAG Bot New tag: 6.1.58 |
|
TAG Bot New tag: 6.1.59 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fly602, pengfeixx The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fix the issue of the storage message click having no effect
Log: Fix the issue of the storage message click having no effect
pms: BUG-333027
Summary by Sourcery
Improve notification interaction in the OBEX agent by converting the requestNotifyCh channel from bool to string, mapping notification action clicks to explicit "receive", "decline", and "obex_cancel" reasons, and adjusting the handler to properly respond to each case. Also increase the file receive notification timeout to 40 seconds.
Bug Fixes:
Enhancements: