Refactor 260702 - #2
Merged
Merged
Conversation
- Replace hardcoded Ubuntu x86_64 include paths with pkg-config discovery (glib-2.0, gio-2.0, gobject-2.0, mm-glib, portaudio-2.0, alsa); CC, PKG_CONFIG, ASTERISK_INCLUDE, MODULES_DIR, ASTETCDIR, DESTDIR all overridable for cross builds. - Link with $(CC) -shared and a version script exporting only the module self-symbol; add -MMD dependency tracking, -Wall -Wextra, install target. - Delete res_mmsd.c: it was compiled with chan_modemmanager's AST_MODULE_SELF_SYM and both modules exported colliding non-static globals (dbus/loop/ptmainloop) with AST_MODFLAG_GLOBAL_SYMBOLS set. MMS returns as a native subsystem inside chan_modemmanager in a later commit, without the mmsd-tng/session-dbus dependency. - Fix the two compile blockers under modern GCC defaults (-Wincompatible-pointer-types as error): ao2_callback teardown helpers now use the correct ao2_callback_fn signature (also clearing device to avoid double-unref with the destructor), and mm_call_send_dtmf gets its GAsyncReadyCallback without the bogus G_CALLBACK cast. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Capture: one blocking-poll thread per active call (snd_pcm_wait with a 100ms timeout + snd_pcm_readi), queueing one 20ms voice frame per period; stop_stream only sets a flag and joins, so no cross-thread snd_pcm calls are needed. Capture streams are explicitly started (and restarted after xrun recovery) since snd_pcm_wait never auto-starts them -- verified against snd-aloop. - Playback: inline snd_pcm_writei from modemmanager_write with xrun/suspend recovery and a single retry. - Devices are opened at channel creation, requesting exactly 8000 then 16000 Hz; the channel format follows whichever rate opened, so the old bug where a card with any other default rate silently produced ast_format_none is structurally gone. Both opens and both param sets are error-checked (the PortAudio version discarded the first open's result). Prefer plug-capable device strings so the ALSA plug layer absorbs 48k-native USB codecs. - input_device/output_device config values are now ALSA PCM names (documented breaking change; key names unchanged). - CLI lists ALSA PCM devices via snd_device_name_hint. - Drop portaudio from the Makefile, CI deps and MODULEINFO. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
New audio_detect.c (pure libc, no Asterisk/GLib deps) extracts the kernel's busnum-port[.port...] USB device address from both the modem's physical device path (mm_modem_get_device) and each /sys/class/sound/cardN/device parent chain; the deepest address wins so hubs don't false-match, interface components (1-2:1.4) are ignored, and a usbN root component is required. Exactly one matching card resolves to plughw:N,0, cached per modem; zero or multiple matches log an actionable diagnostic and audio is refused rather than guessed. input_device/output_device now default to "auto" (autodetect); any explicit value, including "default", is used verbatim. Covered by fixture-based host tests (tests/test_audio_detect.c, run via make check, wired into CI): plain device, behind-hub, PCI-only path, missing usbN root, unknown device, and ambiguous two-card cases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Layout: chan_modemmanager.c is module lifecycle only; mm_bus.c owns the D-Bus connection, MMManager and GMainLoop thread; modem.c/sim.c the pvt containers and MM object attachment; channel.c the channel tech; call.c the MMCall lifecycle; sms.c messaging; audio_alsa.c/audio_detect.c the audio backend; cli.c/config.c the rest. One .so, one self-symbol. Threading model: - Private GMainContext instead of the global default context: other modules in the same Asterisk process may use GLib too, and two threads cannot iterate one context. All proxies are bound to it (created under push_thread_default, incl. mm_bus_push/pop_context wraps around list_calls/create_call/list_messages/get_sim) so every signal provably dispatches on our loop thread. - Signal handlers never block: they snapshot and push tasks onto per-modem serializers (ast_threadpool_serializer over a shared pool), preserving per-modem ordering without cross-modem head-of-line blocking. A separate serializer handles MMManager hotplug (object-added/removed), replacing load-time-only device resolution. - pvt lock guards metadata only; owner access is snapshot+chan-ref (modem_grab_owner); ast_pbx_start never runs under a pvt lock. - GObject refs: exactly-once via attach/detach helpers with stored signal-handler ids, disconnect-before-reconnect (reload can no longer double-connect); signal user_data sims are ref'd via closure notify. - Ordered unload: unregister -> unwatch -> detach (joins capture threads) -> quit+join loop thread -> drop manager/bus -> containers. Bug fixes landed with the code they live in: - sms.c: alloca off-by-one stack overflow; find_sim result checked (was checking the wrong vars), ref released on all paths, modem/messaging NULL-guarded. - channel.c: failed mm_call_start_sync now tears the channel down instead of returning it; sim->modem NULL checks; hangup no longer touches pvt state unlocked. - call.c: MMCall is detached and deleted via the TERMINATED task (never leaked); all enum states handled; busy modems reject a second incoming call. - modem.c: jbconf struct assignment (was memcpy of sizeof(pointer)); own_numbers[0] guarded; reload prunes deconfigured pvts (active calls survive until hangup); load_config's unconditional unref of maybe unassigned objects is gone. - pvt_container.h: OBJ_SEARCH_KEY lookups replace the fake stack-pvt hashing trick. - Logging: ast_debug for tracing, ast_verb reserved for real events; g_print-to-stdout removed. Tree builds with zero warnings under -Wall -Wextra. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Quectel-class modems need e.g. AT+QPCMV=1,2 after every boot before their USB audio function carries call audio. MM's Command() D-Bus API is compiled out of default ModemManager builds on every target distro, so commands go directly to an AT port instead — scoped as the bring-up step of the audio side-channel this driver already owns (like the sysfs ALSA autodetect), not a general provisioning interface. Config ([modemN]): init_commands=AT+QPCMV=1,2;... and optional init_port=/dev/ttyUSBx. Collision handling with ModemManager's port ownership, layered: runs only once the MM modem object exists (probing settled) and reaches ENABLED, once per appearance (flag reset on device re-attach); candidates are MM-reported AT ports minus MM's primary port; ports MM holds with TIOCEXCL fail open() and self-identify; every candidate must answer a bare "AT" liveness probe before real commands are sent, so a port with a competing reader is abandoned rather than fought over; we take TIOCEXCL only for the short session. Failures log once — no retry storms. The tty session primitives live in at_tty.c (pure libc) with pty-based unit tests in tests/test_at_tty.c (OK/ERROR/+CME, URC interleaving, split-line and timeout cases), wired into make check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
src/mms/vendor/ carries wsputil.c/.h and mmsutil.c/.h from mmsd-tng
(gitlab.com/kop316/mmsd @ 341117141f8d, GPL-2 like this project) with
upstream copyright headers intact and a provenance block in each file
listing every local change: wsputil.c is byte-for-byte verbatim; the
headers gained include guards + their own includes; mmsutil.c's daemon
log dependency is replaced by vendor_shim.h (DBG -> g_log). One upstream
-Wenum-conversion warning is suppressed for vendor objects only.
src/mms/mms_codec.{h,c} is the boundary the driver consumes:
- mms_codec_wap_push_extract(): defensive WSP Push parse (with and
without a leading GSM UDH; port-addressing IEI 0x04/0x05), accepting
only application/vnd.wap.mms-message (binary 0xBE or literal text) --
SI/SL/OTA pushes and garbage are cleanly rejected. Zero-copy: the
returned body points into the caller's buffer.
- mms_codec_decode_notification()/decode_retrieve(): wrappers over the
vendored mms_message_decode() enforcing the expected message type.
Documented ownership quirk: attachment offset/length index into the
ORIGINAL pdu buffer, which must outlive the decoded struct.
- mms_codec_message_free(): NULL-safe free.
tests/test_mms_codec.c (make check, 3rd host binary): hand-crafted
byte-commented fixtures (push with/without UDH, wrong-port UDH, SI push
rejected, garbage rejected, minimal retrieve-conf with one text/plain
part) plus two fixtures reused from upstream unit/test-mmsutil.c with
provenance notes. Also stop tracking built test binaries.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
contrib/openwrt/asterisk-chan-modemmanager: plain-make package modeled
on asterisk-chan-lantiq; DEPENDS asterisk +glib2 +modemmanager
+alsa-lib +libcurl (dbus arrives via modemmanager); compiles the repo
Makefile with TARGET_CC and the staging pkg-config wrapper against
STAGING_DIR asterisk headers; installs the module to
/usr/lib/asterisk/modules and the sample as /etc/asterisk/
modemmanager.conf (conffile). Source URL/commit are overridable for
local src-link development; header documents the feeds wiring.
debian/: debhelper-compat 13, dh --with asterisk (asterisk-dev's
dh_asterisk resolves the asterisk-abi-<buildsum> dependency via
${asterisk:Depends} -- verified in the built .deb); module path
computed from dpkg-architecture DEB_HOST_MULTIARCH matching Debian's
astmoddir (/usr/lib/<multiarch>/asterisk/modules, verified from
asterisk-config); make check wired into dh_auto_test; copyright
documents the GPL-2 vendored mmsd-tng codec. dpkg-buildpackage -us -uc
-b produces a lintian-clean package.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replaces the mmsd-tng dependency (which requires systemd, unusable on OpenWrt) with an in-driver pipeline on top of the previously vendored codec: - Intake (sms.c -> mms_on_wap_push_sms): binary SMS payloads are parsed as WSP push; non-MMS pushes are ignored, notifications are decoded, deduplicated by (sim, transaction id) and queued. SIMs without an mmsc configured log one notice and leave the SMS on the modem. - Fetch: dedicated MMS worker thread (condvar-paced, never a modem serializer or the GMainLoop thread) drives libcurl transfers with per-SIM proxy/interface/User-Agent/timeout settings; the size cap is enforced inside the write callback because carriers lie about Content-Length; bare-path content locations resolve against the mmsc base; bounded retries with 30s/2m/10m backoff; HTTP 200 bodies are decode-validated (error PDUs wrapped in 200 fail after one retry); X-Mms-Expiry honored. - Delivery: text/plain parts concatenated (SMIL skipped) into one message on the mms_context -> message_context -> context chain; other parts spooled under <mms_spool>/<txn>/ with MMS_ATTACHMENT_COUNT/ FILE_n/TYPE_n, MMS_SUBJECT and MMS_TRANSACTION_ID variables; photo-only MMS delivers a placeholder body instead of vanishing. - Ack: best-effort hand-encoded M-NotifyResp.ind (WAP-209) POSTed to the MMSC so the carrier stops re-sending; failures never affect delivery. - Durability: the notification SMS is deleted from the modem (via the owning modem's serializer) only on terminal states, so a restart re-discovers pending notifications; state itself is memory-only. New per-SIM config: mmsc, mms_proxy, mms_interface, mms_context, mms_spool, mms_max_size, mms_fetch_timeout, mms_max_retries, mms_ack, mms_user_agent (documented in the sample config). tests/test_mms_fetch.c (4th make check binary) exercises the curl layer against a local one-shot HTTP stub: byte-exact roundtrip + decode, mid-stream size-cap abort, non-200 handling, and the ack POST shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nWrt SDK CI jobs CI: debian:trixie dpkg-buildpackage + lintian job, and an OpenWrt 25.12.5 SDK cross-build job (release-pinned feeds, src-linked package, dl/build caching) uploading the built artifacts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
11 findings confirmed by independent verification, all fixed: - critical: module usecount was never taken, so 'module unload' during an active call dlclose'd the module under a live channel -> crash. modemmanager_new() now takes ast_module_ref(AST_MODULE_SELF), released in modemmanager_hangup(). - critical/major (locking contract): the pvt ao2 lock was held across blocking ALSA I/O (open_stream from modemmanager_new/task_call_added/ start_stream; snd_pcm_writei in alsa_write_frame), contradicting the documented invariant. open_stream() now takes its own locks and runs device opens lock-free; callers open before locking; a new dedicated pcm_lock guards handle pointers and the playback write path; the ao2 lock never wraps ALSA I/O anywhere. - major: sim->modem was read bare all over channel.c/call.c/sms.c/ mms_core.c, racing resolve_object()/sim_detach_all() reassignment. New sim_grab_modem() (lock+ref+unlock) used everywhere, with GObject snapshots (device/voice/messaging) taken under the modem lock; modemmanager_new() now receives the caller's grabbed modem instead of re-reading sim->modem. - critical: resolve path dereferenced NULL DeviceIdentifier/ SimIdentifier from modems still initializing; now guarded and skipped until the identifiers appear. - major: PCM handles leaked when channel allocation failed after a successful open; failure paths now stop_stream() to close them. - major: bare-path X-Mms-Content-Location was appended to the full MMSC URL; absolute paths now resolve against the MMSC origin (scheme://host[:port]) and only relative paths append to the base. - major: ubuntu CI job lacked libcurl4-openssl-dev. - major: debhelper build artifacts (including a prebuilt ELF and stale DEBIAN/control) were committed; untracked and gitignored. - minor: test_mms_fetch make rule missed vendor header prerequisites. Tree still builds warning-free; all 68 test assertions pass; the module still exports only its self-symbol. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
It slipped into the step-8 commit before its .gitignore entry existed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
OpenWrt 25.12 packages with APK (USE_APK default y), not opkg. Set an explicit PKG_VERSION so the package packs as APK-conventional 1.0.0-r1 instead of a bare '1' (without PKG_VERSION the version degenerates to PKG_RELEASE alone), and document the .apk output in the README. The conffiles stanza needs no change: include/package-pack.mk translates it to APK's conffiles mechanism. CI artifact globs were already extension-agnostic. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Verified on OpenWrt 25.12.5 x86_64 with an RM500Q-GL: - the asterisk user needs the audio group (/dev/snd for call audio) and dialout (init AT commands); postinst now says so, like chan_dongle's dialout reminder. - ModemManager claims BOTH AT ports on a QMI-controlled RM500Q, so init_commands needs one port freed via an ID_MM_PORT_IGNORE rule (MM parses /lib/udev/rules.d itself on OpenWrt); README carries a working example rule. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…overy) MMS durability depends on the notification SMS staying on the modem until the message reaches a terminal state — but the 'added' signal only fires for NEW messages, so a notification that arrived while the module was not watching (restart, reload, modem replug) was never processed. Verified live on LGU+: a WAP-push landed during a reload window and sat stored, invisible. Every modem (re)appearance now pushes a rescan task onto the modem's serializer: stored RECEIVED messages with binary payloads are fed through the MMS intake (the dedupe cache absorbs repeats); stored text SMS are skipped since they were delivered on arrival. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
libmm-glib reports textless SMS with an EMPTY text string, not NULL, so both the live added-signal path and the stored-message rescan treated WAP-push notifications as text SMS and skipped the MMS intake. Verified against a live LGU+ MMS notification. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ModemManager sometimes fails to delete multipart notification SMS (QMI 'Couldn't delete N parts' quirk, seen live on an RM500Q), so the notification survives on the modem. Previously a delivered/given-up transaction was unlinked from the dedupe container immediately, and the next rescan or carrier re-send re-fetched and re-delivered the same MMS (observed on LGU+). Terminal transactions now stay linked as tombstones until notification expiry plus a 24h grace, swept by the worker loop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
All verified live on LGU+: the MMSC's native AAAA silently black-holes while the A record through the NAT64 prefix (RFC 7050 discovery) serves m-retrieve.conf; MMS metadata reads via MESSAGE_DATA(); OpenWrt dl-cache and apk same-version pitfalls when iterating locally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pvts were created with identifier = the config SECTION name, linked into the ao2 hash under that key, and then store_config overwrote the identifier field with the configured value — corrupting bucket placement and making every reload miss the existing pvt. Each reload therefore recreated pvts (prune + re-resolve churn) and orphaned the live GObject signal closures: verified on hardware, MMModemMessaging 'added' events stopped arriving after any 'module reload' while working fine on a fresh load. build_modem/build_sim now read the section's identifier value up front (sections without one are skipped with a warning), look up existing pvts by it, and link new pvts only after the config vars are stored, so the hash key never mutates post-link. Reload now updates pvts in place. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Verified live: MM emits Call.StateChanged on the bus (dbus-monitor saw three transitions during a driver call) but the standalone MMCall proxy created via mm_modem_voice_create_call_sync never delivered the GObject 'state-changed' signal to the handler, while proxies obtained from the object manager (e.g. messaging 'added') deliver fine. The property-cache path (PropertiesChanged -> notify::state) is a second, independent delivery mechanism, so call_attach now subscribes to both and a last_call_state tracker dedupes whichever arrives; handlers funnel into one handle_call_state() that also guards against stale MMCall instances. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Driver fixes proven on hardware (OpenWrt 25.12.5, RM500Q-GL, LGU+):
- Subscribe to Call StateChanged at the connection level and execute
the subscription on the GLib loop thread via g_main_context_invoke.
Proxies and subscriptions created off-loop bind a dead thread-default
context and never fire: g_main_context_push_thread_default silently
no-ops ('acquired_context' assertion) once g_main_loop_run owns the
context. The broken push/pop context API is removed; call state is
additionally driven from the property cache (notify::state) as belt
and braces.
- Open ALSA playback with SND_PCM_NONBLOCK and drop frames on -EAGAIN.
A blocking snd_pcm_writei wedged the Asterisk bridge thread when the
modem's USB audio function stopped draining ('Exceptionally long
voice queue length'), muting calls.
- OpenWrt package: depend on kmod-usb-audio (/dev/snd) and the
asterisk bridge/codec sub-packages every real call needs
(bridge-simple/native-rtp/softmix, codec-ulaw/alaw/resample) -
without them calls connect but carry no audio ('No translator
path', 'Could not create class basic'). Gate the package with
@AUDIO_SUPPORT @USB_SUPPORT: alsa-lib is @AUDIO_SUPPORT-gated
upstream, so audio-less targets (e.g. at91/sama7) can neither build
nor run the driver.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rewrite the workflow around distro-matched packages: - test: build with -Werror + unit tests + symbol/install checks on the current Ubuntu LTS runner. - deb: dpkg-buildpackage + lintian in ubuntu:24.04 and ubuntu:26.04 containers, each against that release's own asterisk-dev (asterisk 20 and 22), replacing the dead debian:trixie job (Debian stable ships no asterisk since 2023) and the fragile /__w artifact glob. - openwrt: one job per official 25.12 package architecture that can support the driver (32; arm_cortex-a7_vfpv4 is excluded because its only target has no audio support, hence no alsa-lib in its feed). Each job uses the release SDK of a representative target verified to ship kmod-usb-audio, with feeds.conf built from the release's feeds.buildinfo - the exact commits the stock packages were built from - and default package config, matching the phase-2 buildbot by construction. A guard fails at defconfig with a clear message if the package is ever deselected. SDK dl/build_dir/staging cached per arch; OPENWRT_RELEASE is the single bump point. - release: v* tags attach every .deb/.apk to a GitHub Release. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
README now covers what users need: prebuilt-package install for OpenWrt 25.12 and Ubuntu 24.04/26.04 (with the Debian-has-no-asterisk note), one-time permissions setup, annotated minimal configuration, MMS incl. the NAT64 runbook, dialplan usage, a troubleshooting table from failures seen in real testing, and compatibility (including the audio-less-target exclusion). It also states the guarantee that no non-default build options are required anywhere. Build-from-source, architecture/threading notes, packaging gotchas, CI layout, the arch-matrix regeneration procedure (with the full runtime-dependency sweep) and the release procedure move to DEV_README.md, linked from the README. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three defects found running as a 24/7 gateway on real traffic: - Messaging.Added fires when ModemManager creates the SMS object, which can be before all parts are read off the modem (state 'receiving'); the flip to 'received' is only a property change with no second signal. The intake task silently skipped such messages and nothing ever looked again -- observed live with WAP-push MMS notifications arriving during an active voice call (busy QMI channel widens the window). Poll the message up to 15 times at 2s intervals via a new mm_bus_timeout_add() loop-thread timer before giving up. - Delivered text SMS were never deleted from the modem, so storage filled up (29 messages accumulated in two days of testing); a full store eventually rejects new SMS. Delete after successful dialplan queueing; undeliverable messages stay stored. - MMS fetch failures while a voice call is active no longer count against the retry budget, and call termination kicks the worker to make waiting transactions due immediately (mms_kick). With a correctly configured MMS bearer fetches work fine during calls, but when the bearer shares the call's IMS PDN the modem flow-controls the host data path until hangup (measured on RM500Q/LGU+): every attempt failed, and a long call could exhaust the budget and drop the message for good. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Giving up used to delete the notification from the modem, permanently destroying the message: when a broken bearer ate all four attempts, the MMS was unrecoverable even though it still sat on the MMSC (observed live -- a message was lost exactly this way). Keep the SMS unless the notification has expired; a restart's stored-message rescan then retries with a fresh budget. Expired or permanently undecodable notifications are still deleted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
제곧내