Skip to content

Add configurable startup notification sounds and UI controls #24

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.

Sign up for GitHub

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

Merged
exddc merged 8 commits into from
Mar 4, 2026
Merged

Add configurable startup notification sounds and UI controls #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1d0b75e
Remove added notification wav assets and use existing defaults
exddc Mar 3, 2026
9d12055
Merge branch 'main' of github.com:exddc/virtualchime into codex/linea…
exddc Mar 3, 2026
7a51c38
Merge branch 'main' of github.com:exddc/virtualchime into codex/linea…
exddc Mar 3, 2026
45a79c6
fixed lint script
exddc Mar 4, 2026
8141095
implemented review comments
exddc Mar 4, 2026
ea705e8
review comments 2
exddc Mar 4, 2026
991d814
review comments 3
exddc Mar 4, 2026
1cf3630
review comments 4
exddc Mar 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions buildroot/board/raspberrypi0w/rootfs_overlay/etc/chime.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ time_sync_interval=3600
# Supports MQTT wildcard filters (+ and #), e.g. doorbell/2OG/#
ring_topic=doorbell/ring
sound_path=/usr/local/share/chime/ring.wav
notification_success_sound_path=/usr/local/share/chime/test.wav
notification_failure_sound_path=/usr/local/share/chime/ring.wav
volume_bell=80
volume_notifications=70
volume_other=70
Expand Down
2 changes: 2 additions & 0 deletions chime/include/chime/chime_config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ struct ChimeConfig {

std::string ring_topic = "doorbell/ring";
std::string sound_path = "/usr/local/share/chime/ring.wav";
std::string notification_success_sound_path = "/usr/local/share/chime/test.wav";
std::string notification_failure_sound_path = "/usr/local/share/chime/ring.wav";
int volume_bell = 80;
int volume_notifications = 70;
int volume_other = 70;
Expand Down
4 changes: 4 additions & 0 deletions chime/include/chime/chime_service.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,13 @@ class ChimeService final : public vc::mqtt::EventHandler {
void OnMessage(const vc::mqtt::Message& message) override;

private:
enum class NotificationSoundType { kSuccess, kFailure };

void LogWifiState(const WifiState& state) const;
void LogHealth(bool clock_sane);
bool RingTopicMatches(const std::string& message_topic) const;
bool WifiStateIsConnected(const std::optional<WifiState>& state) const;
void PlayNotification(NotificationSoundType type);
Comment on lines +37 to +43
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Run clang-format on this header to fix CI.

chime CI reports clang-format violations for this file.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@chime/include/chime/chime_service.h` around lines 37 - 43, The header has
clang-format style violations; run the project’s clang-format configuration
against this header (or reformat the declarations manually) so the enum
NotificationSoundType and the method declarations LogWifiState, LogHealth,
RingTopicMatches, WifiStateIsConnected, and PlayNotification follow the repo’s
formatting rules (spacing, alignment, and trailing semicolons consistent with
other headers); ensure the file is saved after formatting and re-run CI to
confirm the violations are resolved.

void RecordObservedTopic(const std::string& topic);
void LoadObservedTopics();
bool PersistObservedTopics(std::string* error) const;
Expand Down
2 changes: 2 additions & 0 deletions chime/include/chime/webd_types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ struct CoreConfig {
std::string mqtt_tls_key_file;
std::vector<std::string> mqtt_topics;
std::string ring_topic = "doorbell/ring";
std::string notification_success_sound_path = "/usr/local/share/chime/test.wav";
std::string notification_failure_sound_path = "/usr/local/share/chime/ring.wav";
Comment on lines +29 to +30
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Run clang-format on this header to unblock CI.

chime CI reports clang-format violations for this file.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@chime/include/chime/webd_types.h` around lines 29 - 30, Run clang-format on
chime/include/chime/webd_types.h to fix formatting violations reported by CI;
reformat the header (including the lines defining
notification_success_sound_path and notification_failure_sound_path) so it
conforms to the project’s clang-format rules and commit the updated file.

int volume_bell = 80;
int volume_notifications = 70;
int volume_other = 70;
Expand Down
4 changes: 4 additions & 0 deletions chime/src/config/chime_config.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ constexpr vc::config::Field<ChimeConfig> kConfigFields[] = {
{"heartbeat_topic", vc::config::parse_string<ChimeConfig, &ChimeConfig::heartbeat_topic>, false},
{"ring_topic", vc::config::parse_string<ChimeConfig, &ChimeConfig::ring_topic>, false},
{"sound_path", vc::config::parse_string<ChimeConfig, &ChimeConfig::sound_path>, false},
{"notification_success_sound_path",
vc::config::parse_string<ChimeConfig, &ChimeConfig::notification_success_sound_path>, false},
{"notification_failure_sound_path",
vc::config::parse_string<ChimeConfig, &ChimeConfig::notification_failure_sound_path>, false},
{"volume_bell", vc::config::parse_int<ChimeConfig, &ChimeConfig::volume_bell, 0, 100>, false},
{"volume_notifications", vc::config::parse_int<ChimeConfig, &ChimeConfig::volume_notifications, 0, 100>, false},
{"volume_other", vc::config::parse_int<ChimeConfig, &ChimeConfig::volume_other, 0, 100>, false},
Expand Down
125 changes: 108 additions & 17 deletions chime/src/service/chime_service.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ namespace {
constexpr int kMqttLoopTimeoutMs = 100;
constexpr int kReconnectDelaySeconds = 1;
constexpr int kHealthLogIntervalSeconds = 60;
constexpr int kStartupNotificationTimeoutSeconds = 10;
constexpr int kStartupUnknownWifiTimeoutSeconds = 30;
constexpr std::time_t kMinimumSaneEpoch = 1704067200;
constexpr std::size_t kMaxPayloadLogBytes = 256;
constexpr std::size_t kMaxObservedTopics = 256;
Expand Down Expand Up @@ -105,11 +107,21 @@ int ChimeService::Run(vc::runtime::SignalHandler &signal_handler) {
"s topic=" + config_.heartbeat_topic);
logger_.Info("audio", "enabled=" + vc::util::BoolToString(config_.audio_enabled) +
" ring_topic=" + config_.ring_topic + " sound_path=" + config_.sound_path);
logger_.Info("audio", "notifications success_path=" + config_.notification_success_sound_path +
" failure_path=" + config_.notification_failure_sound_path +
" volume=" + std::to_string(config_.volume_notifications));
logger_.Info("wifi", "monitor interface=" + config_.wifi_interface +
" interval=" + std::to_string(config_.wifi_check_interval) + "s");

if (config_.audio_enabled && vc::util::IsLinux() && !vc::util::FileExists(config_.sound_path)) {
logger_.Warn("audio", "configured sound file does not exist: " + config_.sound_path);
if (config_.audio_enabled && vc::util::IsLinux()) {
const auto validate_audio_file = [this](const std::string &path, const std::string &label) {
if (!vc::util::FileExists(path)) {
logger_.Warn("audio", "configured " + label + " file does not exist or is not readable: " + path);
}
};
validate_audio_file(config_.sound_path, "ring sound");
validate_audio_file(config_.notification_success_sound_path, "notification success sound");
validate_audio_file(config_.notification_failure_sound_path, "notification failure sound");
}

vc::mqtt::ConnectOptions options;
Expand Down Expand Up @@ -137,18 +149,36 @@ int ChimeService::Run(vc::runtime::SignalHandler &signal_handler) {
auto last_wifi_check = last_heartbeat;
std::optional<WifiState> last_wifi_state;

if (config_.wifi_check_interval > 0) {
const auto wifi_state = wifi_monitor_.ReadState(config_.wifi_interface);
if (wifi_state.has_value()) {
LogWifiState(*wifi_state);
last_wifi_state = wifi_state;
} else {
logger_.Info("wifi", "monitor disabled on non-Linux platform");
}
const auto startup_wifi_state = wifi_monitor_.ReadState(config_.wifi_interface);
if (startup_wifi_state.has_value()) {
LogWifiState(*startup_wifi_state);
last_wifi_state = startup_wifi_state;
} else {
logger_.Info("wifi", "monitor disabled by config");
logger_.Info("wifi", "monitor unavailable on this platform");
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if (config_.wifi_check_interval <= 0) {
logger_.Info("wifi", "periodic monitor disabled by config");
}

const bool startup_notifications_enabled = config_.audio_enabled;
bool startup_notification_played = !startup_notifications_enabled;
bool startup_notification_unknown_logged = false;
std::optional<std::chrono::steady_clock::time_point> startup_unknown_wifi_begin;
const auto startup_begin = std::chrono::steady_clock::now();
const auto refresh_wifi_state = [this, &last_wifi_state]() {
const auto current_state = wifi_monitor_.ReadState(config_.wifi_interface);
if (current_state.has_value()) {
if (WifiStateChanged(last_wifi_state, *current_state)) {
LogWifiState(*current_state);
}
last_wifi_state = current_state;
} else if (last_wifi_state.has_value()) {
logger_.Warn("wifi", "state unavailable; connectivity unknown");
last_wifi_state = std::nullopt;
}
};

while (!signal_handler.ShouldStop()) {
const int loop_rc = mqtt_client_.Loop(kMqttLoopTimeoutMs, 1);
if (signal_handler.ShouldStop()) {
Expand All @@ -168,6 +198,7 @@ int ChimeService::Run(vc::runtime::SignalHandler &signal_handler) {
}

const auto now = std::chrono::steady_clock::now();
bool wifi_state_refreshed = false;

if (config_.heartbeat_interval > 0) {
const auto heartbeat_elapsed =
Expand All @@ -187,15 +218,55 @@ int ChimeService::Run(vc::runtime::SignalHandler &signal_handler) {
if (config_.wifi_check_interval > 0) {
const auto wifi_elapsed = std::chrono::duration_cast<std::chrono::seconds>(now - last_wifi_check).count();
if (wifi_elapsed >= config_.wifi_check_interval) {
const auto current_state = wifi_monitor_.ReadState(config_.wifi_interface);
if (current_state.has_value() && WifiStateChanged(last_wifi_state, *current_state)) {
LogWifiState(*current_state);
last_wifi_state = current_state;
}
refresh_wifi_state();
wifi_state_refreshed = true;
last_wifi_check = now;
}
}

if (!startup_notification_played) {
if (!wifi_state_refreshed) {
refresh_wifi_state();
}
const bool wifi_state_known = last_wifi_state.has_value();
const bool wifi_connected = wifi_state_known && WifiStateIsConnected(last_wifi_state);
const bool mqtt_connected = mqtt_connected_.load();
const auto startup_elapsed = std::chrono::duration_cast<std::chrono::seconds>(now - startup_begin).count();

if (wifi_connected && mqtt_connected) {
logger_.Info("audio", "startup checks passed (wifi + mqtt), playing success notification");
PlayNotification(NotificationSoundType::kSuccess);
startup_notification_played = true;
} else if (startup_elapsed >= kStartupNotificationTimeoutSeconds) {
if (!wifi_state_known) {
if (!startup_notification_unknown_logged) {
logger_.Info("audio", "startup checks deferred: wifi state unknown");
startup_notification_unknown_logged = true;
startup_unknown_wifi_begin = now;
}

if (startup_unknown_wifi_begin.has_value()) {
const auto unknown_wifi_elapsed =
std::chrono::duration_cast<std::chrono::seconds>(now - *startup_unknown_wifi_begin).count();
if (unknown_wifi_elapsed >= kStartupUnknownWifiTimeoutSeconds) {
logger_.Warn("audio", "startup wifi state remained unknown for " +
std::to_string(kStartupUnknownWifiTimeoutSeconds) +
"s after startup timeout, playing failure notification");
PlayNotification(NotificationSoundType::kFailure);
startup_notification_played = true;
}
}
} else {
startup_unknown_wifi_begin = std::nullopt;
logger_.Warn("audio", "startup checks incomplete within " +
std::to_string(kStartupNotificationTimeoutSeconds) +
"s, playing failure notification");
PlayNotification(NotificationSoundType::kFailure);
startup_notification_played = true;
}
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const auto health_elapsed = std::chrono::duration_cast<std::chrono::seconds>(now - last_health).count();
if (health_elapsed >= kHealthLogIntervalSeconds) {
const bool clock_sane = vc::util::ClockIsSane(kMinimumSaneEpoch);
Expand Down Expand Up @@ -366,6 +437,26 @@ bool ChimeService::PersistObservedTopics(std::string *error) const {
return true;
}

bool ChimeService::WifiStateIsConnected(const std::optional<WifiState> &state) const {
if (!state.has_value()) {
return false;
}
return state->interface_present && state->operstate == "up" && state->carrier == 1;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

void ChimeService::PlayNotification(NotificationSoundType type) {
if (!config_.audio_enabled) {
return;
}

if (type == NotificationSoundType::kSuccess) {
audio_player_.Play(config_.notification_success_sound_path, config_.volume_notifications);
return;
}

audio_player_.Play(config_.notification_failure_sound_path, config_.volume_notifications);
}

void ChimeService::LogWifiState(const WifiState &state) const {
if (!state.interface_present) {
logger_.Warn("wifi", "interface '" + config_.wifi_interface + "' not found");
Expand All @@ -377,7 +468,7 @@ void ChimeService::LogWifiState(const WifiState &state) const {
message += " carrier=" + std::to_string(state.carrier);
}

if (state.operstate == "up" && state.carrier != 0) {
if (state.operstate == "up" && state.carrier == 1) {
logger_.Info("wifi", message);
} else {
logger_.Warn("wifi", message + " (connectivity degraded)");
Expand Down
37 changes: 37 additions & 0 deletions chime/src/webd/config_store.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,27 @@ std::vector<ValidationError> ConfigStore::ValidateRequest(
errors.push_back({"ring_topic", "ring_topic is invalid"});
}

const auto validate_sound_path = [&errors](const std::string& field_name,
const std::string& value) {
if (value.find('\n') != std::string::npos ||
value.find('\r') != std::string::npos) {
errors.push_back(
{field_name, field_name + " must not contain newline characters"});
return;
}

const std::string trimmed = vc::config::trim(value);
if (trimmed.empty() || trimmed.size() > 256) {
errors.push_back(
{field_name, field_name + " must be 1-256 chars after trimming"});
}
};

validate_sound_path("notification_success_sound_path",
request.config.notification_success_sound_path);
validate_sound_path("notification_failure_sound_path",
request.config.notification_failure_sound_path);

if (request.config.volume_bell < 0 || request.config.volume_bell > 100) {
errors.push_back({"volume_bell", "volume_bell must be 0-100"});
}
Expand Down Expand Up @@ -512,6 +533,20 @@ SaveResult ConfigStore::LoadCoreConfigInternal() const {
const std::string ring_topic = ExtractConfigValue(chime_lines, "ring_topic");
config.ring_topic = ring_topic.empty() ? defaults.ring_topic : ring_topic;

const std::string notification_success_sound_path =
ExtractConfigValue(chime_lines, "notification_success_sound_path");
config.notification_success_sound_path =
notification_success_sound_path.empty()
? defaults.notification_success_sound_path
: notification_success_sound_path;

const std::string notification_failure_sound_path =
ExtractConfigValue(chime_lines, "notification_failure_sound_path");
config.notification_failure_sound_path =
notification_failure_sound_path.empty()
? defaults.notification_failure_sound_path
: notification_failure_sound_path;

const std::string volume_bell_raw = ExtractConfigValue(chime_lines, "volume_bell");
int volume_bell = defaults.volume_bell;
ParseInt(volume_bell_raw, 0, 100, &volume_bell);
Expand Down Expand Up @@ -581,6 +616,8 @@ bool ConfigStore::SaveChimeConfig(const SaveRequest& request,
{"mqtt_tls_key_file", request.config.mqtt_tls_key_file},
{"mqtt_topics", JoinCsv(request.config.mqtt_topics)},
{"ring_topic", request.config.ring_topic},
{"notification_success_sound_path", request.config.notification_success_sound_path},
{"notification_failure_sound_path", request.config.notification_failure_sound_path},
{"volume_bell", std::to_string(request.config.volume_bell)},
{"volume_notifications", std::to_string(request.config.volume_notifications)},
{"volume_other", std::to_string(request.config.volume_other)},
Expand Down
36 changes: 36 additions & 0 deletions chime/src/webd/web_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -995,6 +995,10 @@ WebServer::HttpResponse WebServer::HandleGetCoreConfig() {
response.body += "\"mqtt_tls_key_file\":" + JsonString(loaded.snapshot.config.mqtt_tls_key_file) + ",";
response.body += "\"mqtt_topics\":" + SerializeTopics(loaded.snapshot.config.mqtt_topics) + ",";
response.body += "\"ring_topic\":" + JsonString(loaded.snapshot.config.ring_topic) + ",";
response.body += "\"notification_success_sound_path\":" +
JsonString(loaded.snapshot.config.notification_success_sound_path) + ",";
response.body += "\"notification_failure_sound_path\":" +
JsonString(loaded.snapshot.config.notification_failure_sound_path) + ",";
response.body += "\"volume_bell\":" + JsonNumber(loaded.snapshot.config.volume_bell) + ",";
response.body += "\"volume_notifications\":" + JsonNumber(loaded.snapshot.config.volume_notifications) + ",";
response.body += "\"volume_other\":" + JsonNumber(loaded.snapshot.config.volume_other) + ",";
Expand Down Expand Up @@ -1036,6 +1040,10 @@ WebServer::HttpResponse WebServer::HandlePostCoreConfig(const HttpRequest &reque
const auto mqtt_tls_key_file = ReadRequiredString(parsed.value, "mqtt_tls_key_file", &parse_errors);
const auto mqtt_topics = ReadRequiredStringArray(parsed.value, "mqtt_topics", &parse_errors);
const auto ring_topic = ReadRequiredString(parsed.value, "ring_topic", &parse_errors);
const auto notification_success_sound_path =
ReadOptionalString(parsed.value, "notification_success_sound_path", &parse_errors);
const auto notification_failure_sound_path =
ReadOptionalString(parsed.value, "notification_failure_sound_path", &parse_errors);
const auto volume_bell = ReadRequiredInt(parsed.value, "volume_bell", &parse_errors);
const auto volume_notifications =
ReadRequiredInt(parsed.value, "volume_notifications", &parse_errors);
Expand All @@ -1052,6 +1060,18 @@ WebServer::HttpResponse WebServer::HandlePostCoreConfig(const HttpRequest &reque
return response;
}

std::optional<CoreConfigSnapshot> existing_snapshot;
if (!notification_success_sound_path.has_value() ||
!notification_failure_sound_path.has_value()) {
const SaveResult loaded = config_store_.LoadCoreConfig();
if (!loaded.success) {
response.status = 500;
response.body = "{\"error\":\"save_failed\",\"message\":" + JsonString(loaded.error) + "}";
return response;
}
existing_snapshot = loaded.snapshot;
}

save_request.config.wifi_ssid = *wifi_ssid;
save_request.config.mqtt_host = *mqtt_host;
save_request.config.mqtt_port = *mqtt_port;
Expand All @@ -1064,6 +1084,18 @@ WebServer::HttpResponse WebServer::HandlePostCoreConfig(const HttpRequest &reque
save_request.config.mqtt_tls_key_file = *mqtt_tls_key_file;
save_request.config.mqtt_topics = *mqtt_topics;
save_request.config.ring_topic = *ring_topic;
if (notification_success_sound_path.has_value()) {
save_request.config.notification_success_sound_path = *notification_success_sound_path;
} else if (existing_snapshot.has_value()) {
save_request.config.notification_success_sound_path =
existing_snapshot->config.notification_success_sound_path;
}
if (notification_failure_sound_path.has_value()) {
save_request.config.notification_failure_sound_path = *notification_failure_sound_path;
} else if (existing_snapshot.has_value()) {
save_request.config.notification_failure_sound_path =
existing_snapshot->config.notification_failure_sound_path;
}
save_request.config.volume_bell = *volume_bell;
save_request.config.volume_notifications = *volume_notifications;
save_request.config.volume_other = *volume_other;
Expand Down Expand Up @@ -1105,6 +1137,10 @@ WebServer::HttpResponse WebServer::HandlePostCoreConfig(const HttpRequest &reque
response.body += "\"mqtt_tls_key_file\":" + JsonString(saved.snapshot.config.mqtt_tls_key_file) + ",";
response.body += "\"mqtt_topics\":" + SerializeTopics(saved.snapshot.config.mqtt_topics) + ",";
response.body += "\"ring_topic\":" + JsonString(saved.snapshot.config.ring_topic) + ",";
response.body += "\"notification_success_sound_path\":" +
JsonString(saved.snapshot.config.notification_success_sound_path) + ",";
response.body += "\"notification_failure_sound_path\":" +
JsonString(saved.snapshot.config.notification_failure_sound_path) + ",";
response.body += "\"volume_bell\":" + JsonNumber(saved.snapshot.config.volume_bell) + ",";
response.body += "\"volume_notifications\":" + JsonNumber(saved.snapshot.config.volume_notifications) + ",";
response.body += "\"volume_other\":" + JsonNumber(saved.snapshot.config.volume_other) + ",";
Expand Down
Loading