-
-
Notifications
You must be signed in to change notification settings - Fork 374
feat(HikVision): add OpenSound/CloseSound method #7404
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 GuideAdds OpenSound/CloseSound support to the HikVision sample component, including UI buttons, state management, and toast notifications for operation results. Sequence diagram for HikVisions OpenSound and CloseSound interactionssequenceDiagram
actor User
participant HikVisions
participant HikVisionWebPlugin
participant ToastService
rect rgb(230,230,255)
User->>HikVisions: Click OpenSound button
HikVisions->>HikVisionWebPlugin: OpenSound()
HikVisionWebPlugin-->>HikVisions: result (bool)
alt OpenSound success
HikVisions->>HikVisions: _openSoundStatus = true
HikVisions->>HikVisions: _closeSoundStatus = false
HikVisions->>ToastService: Success(...)
else OpenSound failure
HikVisions->>ToastService: Error(...)
end
end
rect rgb(230,255,230)
User->>HikVisions: Click CloseSound button
HikVisions->>HikVisionWebPlugin: CloseSound()
HikVisionWebPlugin-->>HikVisions: result (bool)
alt CloseSound success
HikVisions->>HikVisions: _openSoundStatus = false
HikVisions->>HikVisions: _closeSoundStatus = true
HikVisions->>ToastService: Success(...)
else CloseSound failure
HikVisions->>ToastService: Error(...)
end
end
Updated class diagram for HikVisions component sound controlclassDiagram
class HikVisions {
- SwalService SwalService
- ToastService ToastService
- HikVisionWebPlugin _hikVision
- string _ip
- int _port
- string _user
- string _password
- bool _inited
- int _streamType
- bool _loginStatus
- bool _logoutStatus
- bool _startRealPlayStatus
- bool _stopRealPlayStatus
- bool _openSoundStatus
- bool _closeSoundStatus
- List~SelectedItem~ _analogChannels
- int _channelId
+ OnLogout() Task
+ OnStartRealPlay() Task
+ OnStopRealPlay() Task
+ OnOpenSound() Task
+ OnCloseSound() Task
+ OnInitedAsync(initialized bool) Task
}
class HikVisionWebPlugin {
+ Logout() Task
+ StartRealPlay(streamType int, channelId int) Task
+ StopRealPlay() Task
+ OpenSound() Task~bool~
+ CloseSound() Task~bool~
}
class SwalService {
<<service>>
}
class ToastService {
<<service>>
+ Success(title string, content string) Task
+ Error(title string, content string) Task
}
class SelectedItem {
}
HikVisions --> HikVisionWebPlugin : uses
HikVisions --> SwalService : injects
HikVisions --> ToastService : injects
HikVisions --> SelectedItem : contains
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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 found 1 issue, and left some high level feedback:
- Consider wrapping the
_hikVision.OpenSound()and.CloseSound()calls in try/catch blocks so device or network exceptions can be surfaced to the user via a toast instead of failing silently or breaking the component. - The success/failure toast messages for opening/closing sound duplicate the same title and similar content; consider centralizing these strings (and possibly using localization) to avoid repetition and make future changes easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider wrapping the `_hikVision.OpenSound()` and `.CloseSound()` calls in try/catch blocks so device or network exceptions can be surfaced to the user via a toast instead of failing silently or breaking the component.
- The success/failure toast messages for opening/closing sound duplicate the same title and similar content; consider centralizing these strings (and possibly using localization) to avoid repetition and make future changes easier.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor.Server/Components/Samples/HikVisions.razor.cs:84-93` </location>
<code_context>
await _hikVision.StopRealPlay();
}
+ private async Task OnOpenSound()
+ {
+ var result = await _hikVision.OpenSound();
+ if (result)
+ {
+ _openSoundStatus = true;
+ _closeSoundStatus = false;
+ await ToastService.Success("消息通知", "打开声音成功");
+ }
+ else
+ {
+ await ToastService.Error("消息通知", "打开声音失败");
+ }
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider disabling the buttons during the async sound operations to avoid concurrent calls from rapid user clicks.
Because the `IsDisabled` flags only change after `_hikVision.OpenSound()` / `CloseSound()` complete, rapid clicks can fire overlapping handlers and leave the plugin in an inconsistent state if it isn’t re-entrant. You could instead disable the relevant button(s) before awaiting the call, then restore the flags based on the result to guard against rapid repeated interactions.
Suggested implementation:
```csharp
private async Task OnOpenSound()
{
// Disable both buttons immediately to avoid overlapping operations
_openSoundStatus = true;
_closeSoundStatus = true;
var result = await _hikVision.OpenSound();
if (result)
{
// Sound opened: disable "open", enable "close"
_openSoundStatus = true;
_closeSoundStatus = false;
await ToastService.Success("消息通知", "打开声音成功");
}
else
{
// Operation failed: restore to the pre-call state
// (sound presumed closed: enable "open", disable "close")
_openSoundStatus = false;
_closeSoundStatus = true;
await ToastService.Error("消息通知", "打开声音失败");
}
}
```
To fully implement the suggestion and avoid concurrent sound operations, you should apply the same pattern to the `OnCloseSound` handler (or any other sound-related handlers):
1. At the beginning of `OnCloseSound`, set `_openSoundStatus = true;` and `_closeSoundStatus = true;` to disable both buttons before awaiting `_hikVision.CloseSound()`.
2. After the await:
- On success (sound closed), set `_openSoundStatus = false; _closeSoundStatus = true;` so only "open" is enabled.
- On failure, restore the sound-open state: `_openSoundStatus = true; _closeSoundStatus = false;`.
This ensures that while any sound operation is in progress, the user cannot trigger concurrent calls via rapid clicks.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7404 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 748 748
Lines 32793 32793
Branches 4551 4551
=========================================
Hits 32793 32793
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Pull request overview
This PR adds sound control functionality to the HikVision web camera component by implementing OpenSound and CloseSound methods. The changes enable users to control audio playback during video streaming.
- Adds
OpenSoundandCloseSoundmethods with proper state management and user feedback via toast notifications - Introduces sound control buttons in the UI that are enabled/disabled based on the current playback and sound state
- Updates the HikVision package dependency from version 10.0.4 to 10.0.5 to support the new sound control features
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/BootstrapBlazor.Server/Components/Samples/HikVisions.razor.cs | Adds ToastService injection, sound button state flags, and implements OnOpenSound/OnCloseSound methods with proper state management in existing lifecycle methods |
| src/BootstrapBlazor.Server/Components/Samples/HikVisions.razor | Adds two new buttons for opening and closing sound with appropriate disable logic tied to the sound state flags |
| src/BootstrapBlazor.Server/BootstrapBlazor.Server.csproj | Updates BootstrapBlazor.HikVision package reference from version 10.0.4 to 10.0.5 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Link issues
fixes #7403
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add sound control to the HikVision sample by exposing OpenSound and CloseSound actions with UI controls and toast feedback.
New Features:
Enhancements: