feat: Add beep sound notification on probe failures #54
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
Implement a beep sound notification system for probe failures: - Thread-safe Beeper with mutex protection - 500ms debounce interval to prevent continuous beeping - Non-blocking asynchronous execution using goroutines - ON/OFF toggle functionality - Comprehensive test coverage including concurrent access tests
Integrate beep functionality across the application: - Stats manager plays beep on probe failures - Add ToggleBeep() and IsBeepEnabled() to MetricsSystemManager interface - UI key binding 'b' to toggle beep on/off - Header panel displays beep state (ON/OFF) - Footer panel shows 'b:beep' operation guide - Help modal includes beep toggle documentation - BeepStateProvider interface for UI state access All changes are non-blocking and maintain thread safety.
There was a problem hiding this comment.
Pull Request Overview
This PR implements a beep sound notification system that plays an audible alert when probe failures occur. The feature includes both backend functionality and complete UI integration with user controls.
- Adds new
internal/beeppackage with thread-safe beep sound player featuring 500ms debounce - Integrates beep functionality into stats manager to automatically play sounds on probe failures
- Updates UI with beep state display in header, toggle controls, and help documentation
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/beep/beep.go | Core beep sound implementation with debouncing and thread safety |
| internal/beep/beep_test.go | Comprehensive test suite for beep functionality |
| internal/stats/manager.go | Integration of beep system with probe failure events |
| internal/stats/interface.go | Interface extensions for beep control methods |
| internal/ui/tui/app.go | Key binding for 'b' toggle and help text updates |
| internal/ui/tui/layout.go | Wiring of beep state provider to header panel |
| internal/ui/tui/panels/header_footer.go | UI display of beep state and footer controls |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
- Add GetLastBeepTime() method with mutex protection for thread-safe access - Use time.Time.Equal() for time comparison as recommended by staticcheck - Fix race condition detected in TestDebounce - All tests now pass with -race flag
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Changes
Core Implementation
internal/beep: Thread-safe beep sound player with 500ms debounceKey Features
UI Updates
bkey: Toggle beep on/offTest Results
Related
Implements beep sound notification requirements as specified in BEEP.md