feat(HikVision): add OpenSound/CloseSound method #861
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideAdds sound control support to the HikVision web plugin by wiring new JavaScript sound APIs (open, close, set volume) through to the Blazor component and its JS interop wrapper, with basic state checks and error-code handling. Sequence diagram for HikVision sound control via Blazor and JSsequenceDiagram
actor User
participant HikVisionWebPlugin as HikVisionWebPlugin_cs
participant HikVisionWebPlugin_js as HikVisionWebPlugin_js
participant Hikvision_js as hikvision_js
participant Data
participant WebVideoCtrl
User->>HikVisionWebPlugin: OpenSound()
HikVisionWebPlugin->>HikVisionWebPlugin: check IsLogin && IsRealPlaying
alt not logged in or not playing
HikVisionWebPlugin-->>User: false
else logged in and playing
HikVisionWebPlugin->>HikVisionWebPlugin_js: InvokeAsync openSound(Id)
HikVisionWebPlugin_js->>Hikvision_js: openSound(Id)
Hikvision_js->>Data: get(Id)
Data-->>Hikvision_js: vision with iWndIndex and realPlaying
Hikvision_js->>Hikvision_js: check realPlaying === true
alt not real playing
Hikvision_js-->>HikVisionWebPlugin_js: 101
else real playing
Hikvision_js->>WebVideoCtrl: I_OpenSound(iWndIndex)
alt WebVideoCtrl success
WebVideoCtrl-->>Hikvision_js: success
Hikvision_js-->>HikVisionWebPlugin_js: 0
else WebVideoCtrl throws
WebVideoCtrl-->>Hikvision_js: exception
Hikvision_js->>Hikvision_js: set code = 102
Hikvision_js-->>HikVisionWebPlugin_js: 102
end
end
HikVisionWebPlugin_js-->>HikVisionWebPlugin: code
HikVisionWebPlugin->>HikVisionWebPlugin: ret = (code == 0)
HikVisionWebPlugin-->>User: ret
end
User->>HikVisionWebPlugin: SetVolume(value)
HikVisionWebPlugin->>HikVisionWebPlugin: check IsLogin && IsRealPlaying
alt not logged in or not playing
HikVisionWebPlugin-->>User: false
else logged in and playing
HikVisionWebPlugin->>HikVisionWebPlugin_js: InvokeAsync setVolume(Id, clamped value)
HikVisionWebPlugin_js->>Hikvision_js: setVolume(Id, value)
Hikvision_js->>Data: get(Id)
Data-->>Hikvision_js: vision with iWndIndex and realPlaying
Hikvision_js->>Hikvision_js: check realPlaying === true
alt not real playing
Hikvision_js-->>HikVisionWebPlugin_js: 101
else real playing
Hikvision_js->>Hikvision_js: parse and clamp volume 0-100
Hikvision_js->>WebVideoCtrl: I_SetVolume(volume)
alt WebVideoCtrl success
WebVideoCtrl-->>Hikvision_js: success
Hikvision_js-->>HikVisionWebPlugin_js: 0
else WebVideoCtrl throws
WebVideoCtrl-->>Hikvision_js: exception
Hikvision_js->>Hikvision_js: set code = 101
Hikvision_js-->>HikVisionWebPlugin_js: 101
end
end
HikVisionWebPlugin_js-->>HikVisionWebPlugin: code
HikVisionWebPlugin->>HikVisionWebPlugin: ret = (code == 0)
HikVisionWebPlugin-->>User: ret
end
Class diagram for updated HikVisionWebPlugin sound methodsclassDiagram
class HikVisionWebPlugin {
string Id
bool IsLogin
bool IsRealPlaying
Task TriggerGetChannelList(HikVisionChannel channel)
Task bool OpenSound()
Task bool CloseSound()
Task bool SetVolume(int value)
}
class HikVisionChannel {
}
HikVisionWebPlugin --> HikVisionChannel : uses
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
setVolumethe catch block setscode = 101while the other methods use102for exceptions and101for precondition failures, which makes error semantics inconsistent and harder to interpret; consider standardizing the error codes acrossopenSound,closeSound, andsetVolume. - Volume normalization is currently done both in C# (
Math.Max(0, Math.Min(100, value))) and again in JS (parseInt+ clamp + default 50), which is redundant and can diverge over time; consider centralizing the range/default handling in a single layer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `setVolume` the catch block sets `code = 101` while the other methods use `102` for exceptions and `101` for precondition failures, which makes error semantics inconsistent and harder to interpret; consider standardizing the error codes across `openSound`, `closeSound`, and `setVolume`.
- Volume normalization is currently done both in C# (`Math.Max(0, Math.Min(100, value))`) and again in JS (`parseInt` + clamp + default 50), which is redundant and can diverge over time; consider centralizing the range/default handling in a single layer.
## Individual Comments
### Comment 1
<location> `src/components/BootstrapBlazor.HikVision/wwwroot/hikvision.js:351-352` </location>
<code_context>
}
+export async function openSound(id) {
+ const vision = Data.get(id);
+ const { iWndIndex, realPlaying } = vision;
+
+ if (realPlaying !== true) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Handle the case where `Data.get(id)` returns `undefined` to avoid a runtime error during destructuring.
If `Data.get(id)` can return `undefined` (e.g., after `dispose` or for an invalid `id`), the destructuring will throw before you can return an error. Add a null/undefined check on `vision` (e.g., `if (!vision) return 101;`) before destructuring in `openSound`, `closeSound`, and `setVolume`.
</issue_to_address>
### Comment 2
<location> `src/components/BootstrapBlazor.HikVision/wwwroot/hikvision.js:401-408` </location>
<code_context>
+ try {
+ await WebVideoCtrl.I_OpenSound(iWndIndex);
+ }
+ catch (ex) {
+ code = 102;
+ console.log(ex);
+ }
+ return code;
+}
+
+export async function closeSound(id) {
+ const vision = Data.get(id);
+ const { iWndIndex, realPlaying } = vision;
+
+ if (realPlaying !== true) {
+ return 101;
+ }
+
+ let code = 0;
+ try {
+ await WebVideoCtrl.I_CloseSound(iWndIndex);
+ }
+ catch (ex) {
+ code = 102;
+ console.log(ex);
+ }
+ return code;
+}
+
+export async function setVolume(id, value) {
+ const vision = Data.get(id);
+ const { iWndIndex, realPlaying } = vision;
+
+ if (realPlaying !== true) {
+ return 101;
+ }
+
+ let v = parseInt(value);
+ if (isNaN(v)) {
+ v = 50;
+ }
+
+ let code = 0;
+ try {
+ await WebVideoCtrl.I_SetVolume(Math.min(100, Math.max(0, v)));
+ }
+ catch (ex) {
+ code = 101;
+ console.log(ex);
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Use a distinct error code in `setVolume`'s catch block to keep semantics consistent with `openSound`/`closeSound`.
`openSound`/`closeSound` use `101` for "not playing" and `102` for exceptions, but `setVolume` uses `101` for both. This makes it impossible for callers to distinguish an invalid state from a runtime failure. Consider using `102` (or another distinct code) in the `catch` block for consistency and clearer error handling.
```suggestion
let code = 0;
try {
await WebVideoCtrl.I_SetVolume(Math.min(100, Math.max(0, v)));
}
catch (ex) {
code = 102;
console.log(ex);
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR adds sound control functionality to the HikVision Web Plugin component by implementing three new methods: OpenSound, CloseSound, and SetVolume. The version number is incremented from 10.0.4 to 10.0.5.
- Adds JavaScript functions for sound control operations with error handling
- Exposes sound control methods in the C# API layer with proper validation
- Updates module exports to include the new functionality
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
wwwroot/hikvision.js |
Adds three new exported functions (openSound, closeSound, setVolume) with error handling and validation |
Components/HikVisionWebPlugin.razor.js |
Updates imports and exports to include the new sound control functions |
Components/HikVisionWebPlugin.razor.cs |
Adds public OpenSound, CloseSound, and SetVolume methods with proper state checks and JavaScript interop |
BootstrapBlazor.HikVision.csproj |
Increments package version to 10.0.5 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await WebVideoCtrl.I_SetVolume(Math.min(100, Math.max(0, v))); | ||
| } | ||
| catch (ex) { | ||
| code = 102; |
There was a problem hiding this comment.
Inconsistent error code in the setVolume function. The catch block uses error code 101, while the openSound and closeSound functions use 102 for operation failures. For consistency with the other sound control functions, this should be 102.
Comment on lines
+350
to
+410
| export async function openSound(id) { | ||
| const vision = Data.get(id); | ||
| const { iWndIndex, realPlaying } = vision; | ||
|
|
||
| if (realPlaying !== true) { | ||
| return 101; | ||
| } | ||
|
|
||
| let code = 0; | ||
| try { | ||
| await WebVideoCtrl.I_OpenSound(iWndIndex); | ||
| } | ||
| catch (ex) { | ||
| code = 102; | ||
| console.log(ex); | ||
| } | ||
| return code; | ||
| } | ||
|
|
||
| export async function closeSound(id) { | ||
| const vision = Data.get(id); | ||
| const { iWndIndex, realPlaying } = vision; | ||
|
|
||
| if (realPlaying !== true) { | ||
| return 101; | ||
| } | ||
|
|
||
| let code = 0; | ||
| try { | ||
| await WebVideoCtrl.I_CloseSound(iWndIndex); | ||
| } | ||
| catch (ex) { | ||
| code = 102; | ||
| console.log(ex); | ||
| } | ||
| return code; | ||
| } | ||
|
|
||
| export async function setVolume(id, value) { | ||
| const vision = Data.get(id); | ||
| const { iWndIndex, realPlaying } = vision; | ||
|
|
||
| if (realPlaying !== true) { | ||
| return 101; | ||
| } | ||
|
|
||
| let v = parseInt(value); | ||
| if (isNaN(v)) { | ||
| v = 50; | ||
| } | ||
|
|
||
| let code = 0; | ||
| try { | ||
| await WebVideoCtrl.I_SetVolume(Math.min(100, Math.max(0, v))); | ||
| } | ||
| catch (ex) { | ||
| code = 102; | ||
| console.log(ex); | ||
| } | ||
| return code; | ||
| } |
There was a problem hiding this comment.
The three new sound control functions (openSound, closeSound, setVolume) have duplicated validation and error handling logic. Consider extracting this into a shared helper function that takes the operation function as a parameter to improve maintainability and reduce code duplication.
|
|
||
| export async function setVolume(id, value) { | ||
| const vision = Data.get(id); | ||
| const { iWndIndex, realPlaying } = vision; |
There was a problem hiding this comment.
Unused variable iWndIndex.
Suggested change
| const { iWndIndex, realPlaying } = vision; | |
| const { realPlaying } = vision; |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #860
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add sound control capabilities to the HikVision web plugin, including opening, closing, and adjusting volume during real-time playback.
New Features:
Enhancements: