Add ring broadcast, ring timeout watchdog, CMTI dedup, and hangup fix

[KerseyFabrications]

• Broadcast ring event on each subsequent RING URC (enables DAWN ringtone + MIRAGE notification TTL reset)
• Ring timeout watchdog: polls AT+CLCC after 10s of no RING to detect voicemail forwarding without termination URC
• CMTI dedup: suppress duplicate +CMTI for same index within 3s
• Hangup now publishes call_ended directly with reason "local_hangup" instead of relying on URC (fixes stuck MIRAGE notification)

[qodo-code-review]

Review Summary by Qodo

Add ring broadcast, timeout watchdog, CMTI dedup, and hangup fix

✨ Enhancement 🐞 Bug fix

Walkthroughs

Description

• Broadcast ring event on each subsequent RING URC for MIRAGE/DAWN
• Ring timeout watchdog polls AT+CLCC after 10s to detect voicemail forwarding
• CMTI deduplication suppresses duplicate SMS notifications within 3s window
• Hangup publishes call_ended directly instead of relying on URC

Diagram

flowchart LR
  RING["RING URC received"]
  RING -- "update g_last_ring_time" --> STATE{Call state?}
  STATE -- "RINGING_IN" --> BROADCAST["Broadcast ring event"]
  STATE -- "IDLE" --> NEWCALL["Set RINGING_IN state"]
  BROADCAST --> MQTT1["Publish to MQTT"]
  NEWCALL --> MQTT1
  
  HANGUP["Hangup command"]
  HANGUP -- "AT+CHUP" --> IDLE["Set IDLE state"]
  IDLE -- "publish call_ended" --> MQTT2["Publish to MQTT"]
  
  WATCHDOG["Ring timeout watchdog"]
  WATCHDOG -- "10s elapsed" --> CLCC["Poll AT+CLCC"]
  CLCC -- "no calls" --> TIMEOUT["Publish ring_timeout"]
  TIMEOUT --> MQTT3["Publish to MQTT"]
  
  CMTI["CMTI URC received"]
  CMTI -- "check dedup" --> DEDUP{Duplicate<br/>within 3s?}
  DEDUP -- "yes" --> SUPPRESS["Suppress"]
  DEDUP -- "no" --> PROCESS["Process SMS"]

Loading

File Changes

1. src/oasis-echo.c Enhancement, bug fix +70/-2

Ring broadcast, timeout watchdog, CMTI dedup, hangup fix

• Added ring timeout watchdog with AT+CLCC polling to detect voicemail forwarding after 10s of no
 RING URC
• Modified RING URC handler to broadcast lightweight ring event on subsequent RINGs for MIRAGE/DAWN
 notification TTL reset
• Implemented CMTI deduplication logic to suppress duplicate SMS notifications for same index within
 3s window
• Enhanced hangup command to publish call_ended event directly with "local_hangup" reason instead of
 relying on URC
• Added g_last_ring_time tracking and reset logic across call state transitions

src/oasis-echo.c

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1) 📎 Requirement gaps (0)

1. mqtt_publish_event() return ignored 📘 Rule violation ≡ Correctness

Description

Newly added event publishes ignore the int return value from mqtt_publish_event(), so publish
failures can be silently lost. This violates the requirement to check/handle function return values
instead of ignoring them.

Code

src/oasis-echo.c[R198-200]

+            if (mqtt_build_event_json("ring", NULL, json, sizeof(json)) >= 0) {
+               mqtt_publish_event(json);
+            }

Evidence

PR Compliance ID 343436 requires that functions returning status values be checked/used;
mqtt_publish_event() returns int but the newly added calls invoke it as a standalone statement
without checking success/failure.

Rule 343436: Check and handle function return values instead of ignoring them
src/oasis-echo.c[198-200]
src/oasis-echo.c[508-510]
src/oasis-echo.c[917-919]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New code calls `mqtt_publish_event(json)` without checking its `int` return value. This can hide failed publishes and violates the return-value handling compliance rule.

## Issue Context
`mqtt_publish_event()` is declared as `int mqtt_publish_event(const char *event_json);`, so it should be checked for success/failure and handled (e.g., log via `OLOG_WARNING`/`OLOG_ERROR`, retry, or propagate an error depending on the call site).

## Fix Focus Areas
- src/oasis-echo.c[198-200]
- src/oasis-echo.c[508-510]
- src/oasis-echo.c[917-919]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Unsynced ring timestamp access 🐞 Bug ☼ Reliability

Description

g_last_ring_time is written from the URC reader thread and read/overwritten from the main thread
without atomic operations or locking, which is undefined behavior and can cause the ring watchdog to
misfire or never fire. This is inconsistent with g_call_state, which is explicitly made thread-safe
via __atomic builtins.

Code

src/oasis-echo.c[R905-929]

+      if (get_call_state() == CALL_STATE_RINGING_IN && g_last_ring_time > 0 &&
+          (now - g_last_ring_time) >= RING_TIMEOUT_SEC) {
+         /* Ask the modem if any calls are active (AT+CLCC lists current calls) */
+         at_response_t clcc_resp;
+         at_status_t clcc_rc = at_command_send(&g_at_ctx, "AT+CLCC", &clcc_resp, 3000);
+         if (clcc_rc == AT_OK && strstr(clcc_resp.data, "+CLCC:") == NULL) {
+            /* Modem confirms no active calls — carrier forwarded to voicemail */
+            set_call_state(CALL_STATE_IDLE);
+            g_last_ring_time = 0;
+            char json[1024];
+            struct json_object *extra = json_object_new_object();
+            json_object_object_add(extra, "reason", json_object_new_string("ring_timeout"));
+            if (mqtt_build_event_json("call_ended", extra, json, sizeof(json)) >= 0) {
+               mqtt_publish_event(json);
+            }
+            json_object_put(extra);
+            OLOG_INFO("Ring timeout: modem confirms no active calls after %ds", RING_TIMEOUT_SEC);
+         } else if (clcc_rc == AT_OK) {
+            /* Call still active on modem — extend the timeout */
+            g_last_ring_time = now;
+            OLOG_INFO("Ring timeout check: modem reports call still active, extending");
+         } else {
+            /* AT command failed — extend timeout, don't make assumptions */
+            g_last_ring_time = now;
+            OLOG_WARNING("Ring timeout check: AT+CLCC failed (rc=%d), extending", clcc_rc);

Evidence

on_urc_event() is explicitly documented as running on the URC reader thread and updates
g_last_ring_time, while the main loop’s ring timeout watchdog reads/writes the same global and
process_mqtt_command() also writes it; no synchronization is used for g_last_ring_time unlike
g_call_state.

src/oasis-echo.c[49-64]
src/oasis-echo.c[186-203]
src/oasis-echo.c[490-498]
src/oasis-echo.c[887-931]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`g_last_ring_time` is accessed from both the URC reader thread (`on_urc_event`) and the main thread (ring watchdog and hangup path) without synchronization. In C, this is a data race (undefined behavior) and can lead to incorrect watchdog decisions.

## Issue Context
The code already treats `g_call_state` as thread-safe using `__atomic_*` builtins; `g_last_ring_time` should follow the same pattern or be confined to a single thread.

## Fix Focus Areas
- src/oasis-echo.c[49-64]
- src/oasis-echo.c[186-203]
- src/oasis-echo.c[490-498]
- src/oasis-echo.c[887-931]

## Suggested fix
1. Add atomic helpers similar to `get_call_state()/set_call_state()`:
  - `static time_t get_last_ring_time(void) { return __atomic_load_n(&g_last_ring_time, __ATOMIC_ACQUIRE); }`
  - `static void set_last_ring_time(time_t t) { __atomic_store_n(&g_last_ring_time, t, __ATOMIC_RELEASE); }`
2. Replace all direct reads/writes of `g_last_ring_time` with these helpers.

(Alternative: push a "ring_seen" command into the main thread queue and only mutate `g_last_ring_time` on the main thread.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Hangup can miss call_ended 🐞 Bug ≡ Correctness

Description

process_mqtt_command() sets CALL_STATE_IDLE unconditionally after AT+CHUP and only publishes
call_ended when rc == AT_OK; if rc is any other terminator (e.g., AT_NO_CARRIER), call_ended is not
emitted and later URC_NO_CARRIER is suppressed because the state is already IDLE. This can
reintroduce the exact “stuck notification / missing call_ended” failure mode for non-OK hangup
completions.

Code

src/oasis-echo.c[R492-516]

+      call_state_t prev = get_call_state();
      set_call_state(CALL_STATE_HANGING_UP);
      at_response_t resp;
      at_status_t rc = at_command_send(&g_at_ctx, "AT+CHUP", &resp, AT_TIMEOUT_DEFAULT);
      set_call_state(CALL_STATE_IDLE);
+      g_last_ring_time = 0;
      if (rc == AT_OK) {
         mqtt_publish_response(action, request_id, true, NULL, NULL, NULL);
+         /* Publish call_ended directly — don't rely on URC which we'll suppress
+          * since state is already IDLE by the time it arrives. */
+         if (prev != CALL_STATE_IDLE) {
+            char json[1024];
+            struct json_object *extra = json_object_new_object();
+            json_object_object_add(extra, "reason",
+                                   json_object_new_string("local_hangup"));
+            json_object_object_add(extra, "duration", json_object_new_int(0));
+            if (mqtt_build_event_json("call_ended", extra, json, sizeof(json)) >= 0) {
+               mqtt_publish_event(json);
+            }
+            json_object_put(extra);
+            OLOG_INFO("Call ended: local hangup");
+         }
      } else {
         mqtt_publish_response(action, request_id, false, NULL, at_status_str(rc), "Hangup failed");
      }

Evidence

at_command_send can return non-AT_OK statuses when the modem sends terminators like "NO CARRIER"
(mapped to AT_NO_CARRIER). The hangup path currently publishes call_ended only on AT_OK yet sets the
state to IDLE regardless, and URC_NO_CARRIER explicitly suppresses publishing when already IDLE—so a
non-OK terminator can prevent any call_ended publication.

src/oasis-echo.c[230-246]
src/oasis-echo.c[490-516]
src/at_command.c[444-477]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The hangup flow sets the call state to `IDLE` regardless of `AT+CHUP` result, but publishes `call_ended` only when `rc == AT_OK`. For other terminator statuses (notably `AT_NO_CARRIER`), this can suppress any later URC-based call end event and leave subscribers without a `call_ended` notification.

## Issue Context
- `at_parse_terminator()` explicitly treats `NO CARRIER` as a terminator and maps it to `AT_NO_CARRIER`, so `at_command_send()` can return a non-OK status even when the call actually ended.
- `on_urc_event()` suppresses `URC_NO_CARRIER` when the call state is already `IDLE`.

## Fix Focus Areas
- src/oasis-echo.c[490-516]
- src/oasis-echo.c[230-246]
- src/at_command.c[444-477]

## Suggested fix
Option A (recommended):
1. Don’t transition to `CALL_STATE_IDLE` unless you have strong evidence the call ended.
  - Treat `AT_OK` and `AT_NO_CARRIER` as “hangup completed”.
  - If `rc` is something else (e.g. `AT_TIMEOUT`, `AT_PORT_ERROR`, generic `AT_ERROR`), restore `prev` (or keep `CALL_STATE_HANGING_UP`) so that subsequent URCs are not suppressed.
2. Publish `call_ended` for both `AT_OK` and `AT_NO_CARRIER` when `prev != CALL_STATE_IDLE`.

Option B:
- Always publish `call_ended` when `prev != IDLE` after sending `AT+CHUP`, but include the actual `rc` in the event payload so consumers can distinguish “confirmed” vs “best effort”.

Also ensure `g_last_ring_time` reset aligns with the same success conditions (or with the state transition to `IDLE`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more