macos dev compatibility#1
Conversation
Extract the Linux-only RT primitives (SCHED_FIFO, SCHED_RR, CPU affinity, clock_nanosleep) into a shared oals::rt namespace under netutils/platform/. Linux body is a byte-for-byte lift of the helpers previously inlined in engine/main.cpp and io_sim/main.cpp (same priorities, same affinities, same syscalls). macOS body is silent no-ops plus nanosleep-based precise_sleep_ns — best-effort dev target, no QoS or mach_wait_until per plan decision. Gated on NOT EMBEDDED_BUILD so Zephyr firmware compiles zero bytes of rt.cpp; verified by configuring with -DEMBEDDED_BUILD=ON and confirming rt.cpp.o is not produced. Unblocks engine/io_sim compilation on macOS (linking still depends on M3 transport seam). Linux behavior unchanged: same SCHED_FIFO priorities (25/20/80 in engine, 50 in io_sim), same SCHED_RR priority (99 in io_sim), same CPU affinities (1/1/2/3 in engine), same CLOCK_MONOTONIC sleeps. See OpenAudioLiveSystem/Docs/dev-tooling-plan.md M1.
…d (M3 of dev-tooling plan)
Introduce ITransport abstraction in netutils/transport/ so LowLatSocket
and NetworkMapper no longer call AF_PACKET sendto/recv or Zephyr's
extern "C" hooks directly. The dispatch is selected at runtime by
init_socket parsing the iface string for a sim:<name> or raw:<ifname>
prefix (or OAN_TRANSPORT env var). This unblocks the engine binary
from linking on macOS, which was the M2 link gap.
Three backends ship:
- RawLinuxTransport: verbatim lift of the AF_PACKET code from
LowLatSocket.cpp's previous Linux arm. Used only when a raw:
prefix is passed (opt-in on Linux; the no-prefix default path
keeps the existing direct AF_PACKET fast path, unchanged).
- SimTransport: STUB for M3 (open() prints "not yet implemented
(M4)" and returns false). M4 ships the Unix-socket daemon client.
- RawMacTransport: STUB only (BPF deferred to Track 2).
Gating shape across LowLatSocket.{h,cpp} and NetworkMapper.cpp:
#if defined(__linux__) direct AF_PACKET (default, unchanged)
or ITransport when m_transport set
#elif defined(OAN_HOST_BACKENDS) ITransport always; no prefix → fail
#else Zephyr extern "C" hooks (unchanged)
Linux's default no-prefix path is byte-for-byte unchanged: backend_send
falls through to the same sendto(MSG_DONTWAIT, &m_iface_addr, ...) as
pre-M3; backend_recv to the same recv(MSG_DONTWAIT). Templates'
INT_LLP framing and MAC lookup are unchanged.
Other changes:
- htons macro at LowLatSocket.h:38 now three-way: OAN_HOST_BACKENDS
→ <arpa/inet.h>; !__ZEPHYR__ → existing byte-swap macro; else →
zephyr/net/net_ip.h. Skips the collision with the real htons
pulled in by arpa/inet.h on Mac host.
- Fix the _fetch_iface_meta ODR hazard the dev-tooling plan flagged:
standardize to const std::string& everywhere on the OAN side
(LowLatSocket.cpp:88 used to declare it non-const while
NetworkMapper.cpp:12 declared it const — same extern "C" symbol,
two prototypes). Firmware-side definitions in IO_Board_FW and
RackedIO_Board_FW still use non-const std::string& but are
extern "C" so the linker symbol is identical; follow-up PRs to
those repos to harmonize.
- NetworkMapper local_now/local_now_us chrono path extended to
include OAN_HOST_BACKENDS (avoids needing _now_ms/_now_us host
stubs). _delay declaration dropped (was unreferenced).
- extern "C" hook declarations wrapped in
#if !defined(__linux__) && !defined(OAN_HOST_BACKENDS) so they are
Zephyr-only.
- LowLatSocket Linux dtor now guards close(m_socket) on m_socket > 0
(previously closed fd 0 / stdin if init_socket was never called).
CMake: transport/* sources gated under OAN_HOST_BACKENDS. RawLinux
only added when NOT APPLE (BPF placeholder is the Mac counterpart).
target_link_options(oannetutils PRIVATE LINKER:-undefined,dynamic_lookup)
on APPLE to defer NetworkMapper::get_mac_by_uid symbol resolution
to the executable link (Apple ld64 is strict about undefined symbols
in shared libs; Linux ld is lenient — circular dep oannetutils →
NetworkMapper → oancommon → oannetutils is otherwise the same issue
both platforms would face).
Known follow-ups (M4 / Track 2 scope, not blocking):
- NetworkMapper::update_packet calls host_iface_meta before
init_socket has a chance to pick SimTransport, so passing
sim:default on Mac prints a "no AF_LINK entry" warning before
SimTransport's own stub message fires. M4 needs to plumb the
transport's synthetic MAC into update_packet (likely by making
PeerConf or the mapper consult the transport for self-MAC).
- Templates now funnel through non-template backend_send /
backend_recv defined out-of-line in oannetutils.so. Adds one
call boundary per packet vs. pre-M3. Negligible in cold paths;
worth benchmarking on the engine's pipe updater tick before
sound-on-target sessions. -flto would resolve.
Verified on macOS: full build (100% Built target, 11 binaries +
libraries), 0 compile errors. Smoke runs: ./OALSEngine en99 prints
"host_iface_meta: no AF_LINK entry" + "this host requires a prefix"
and exits; ./OALSEngine sim:default prints "SimTransport: not yet
implemented (M4)" and exits. Linux byte-for-byte regression verified
by simulating cmake -DOAN_HOST_BACKENDS=OFF (no transport sources
compiled, no arpa/inet.h pulled in, no -undefined dynamic_lookup
flag, oannetutils builds cleanly).
See OpenAudioLiveSystem/Docs/dev-tooling-plan.md M3.
Replaces the M3 stub with a real AF_UNIX SOCK_STREAM client that speaks the sim_switch daemon's length-prefixed framing. Adds: - SimHello / SimFrame wire structs (local copy; canonical contract lives in OALS tools/sim_switch/sim_proto.h — must stay byte-identical). - Versioned hello (SIM_VERSION=1) with mismatch close on the daemon side. - Partial-read state machine across recv() calls, honouring both async (return 0 on EAGAIN) and blocking (poll-and-retry) modes. - ',mac=02:..' URI suffix to pin a synthetic locally-administered MAC; random fallback minted once per process so all four engine sockets share one device identity (OAN_SIM_MAC env var overrides). - writev/sendmsg for atomic hdr+payload; MSG_DONTWAIT + MSG_NOSIGNAL (Linux only); idempotent SIGPIPE ignore for macOS. - Pre-reserved m_body_buf to avoid per-frame allocation on the audio hot path. - Oversize-frame guard with diagnostic stderr. NetworkMapper now sources self_address from LowLatSocket::get_self_mac() under OAN_HOST_BACKENDS instead of the broken host_iface_meta lookup on transport-prefix interfaces. Idempotent: re-running update_packet after init_mapper preserves the MAC.
packet_sender and mapper_process were #ifdef __linux__ around their usleep pacing, leaving Mac (OAN_HOST_BACKENDS) to busy-spin both loops. Result: 99k disco msg/s and 11 MiB/s of broadcast traffic blowing past sim_switch's per-conn write buffers (60M+ drops in seconds). Extend the gate to also fire on OAN_HOST_BACKENDS so Mac gets the same 5s send / 1s age cadence as Linux. Embedded Zephyr (no host backends, no threads) stays untouched.
SimFrame now carries src_uid populated switch-side, so promiscuous clients (oaninspect) can see who sent each frame. SimHello._reserved becomes flags so the same client can opt into promiscuous mirroring. Header buffer grows 8 → 12 bytes to match SimFrame v2 layout. Engine clients zero-init src_uid on send; the switch overwrites with the sender's hello uid.
Introduces a wait_readable(timeout_ms) primitive on ITransport and plumbs it through LowLatSocket so callers can block on a fd until data arrives or a timeout elapses. Default impl returns 1 immediately so the Linux AF_PACKET RT path keeps its tight-loop semantics byte-identical; SimTransport overrides with poll(POLLIN, timeout_ms). ClockMaster / ClockSlave gain wait_sync_readable() helpers that delegate to the underlying socket. Lets OALS's clock-recv threads stop busy-polling on EAGAIN over AF_UNIX without changing the recv contract. SimTransport::recv is unchanged.
oals::rt::set_thread_realtime was a no-op on macOS. Replace with thread_policy_set(THREAD_TIME_CONSTRAINT_POLICY): map the Linux SCHED_FIFO priority (5..100) to a fraction of one 64-sample audio block (~667 µs at 96 kHz), clamped to [50 µs, 500 µs]. The kernel treats this as "wake within `period`, give us up to `computation` of CPU, must finish within `constraint`" — same overrun semantics as SCHED_FIFO (we just run later, not killed). set_process_scheduler_rr on Mac becomes the natural one-time spot for mlockall(MCL_CURRENT|MCL_FUTURE) so RT stacks can't page out under load. Soft-fails with a warning since macOS returns ENOSYS for non-root processes; engine still runs fine without it. set_running_cpu stays a no-op: thread_affinity_policy_set is documented as a hint and is effectively unhonored on Apple Silicon. Verified: engine's four RT threads now show PRI=97R (high-priority time-constraint) in ps -M while the workqueue + mainline threads stay at PRI=31T. Idle CPU still 0.9 % (same as post-M6).
CI: GitHub Actions matrix runs on every push and PR to any branch, across (ubuntu, OAN_HOST_BACKENDS=OFF), (ubuntu, host=ON), and (macos-latest, host=ON). The host=OFF Linux job keeps the Zephyr- shaped firmware path green; host=ON exercises SimTransport. clang-format: LLVM-based with K&R braces, 100-col limit, left-aligned references to match the codebase's auto& majority. Apply via editor on save — do NOT mass-format existing files. pre-commit: clang-format + trailing-whitespace + EOF fixer + merge- conflict + large-file guard. Install via 'pip install pre-commit && pre-commit install'.
The host-backend define was only injected by the parent OALS CMakeLists. That meant standalone OAN builds (e.g. CI on this repo) would compile with OAN_HOST_BACKENDS undefined regardless of the -D flag — and then NetworkMapper.cpp's #else branch would reference firmware externs (_now_ms, _now_us, _fetch_iface_meta), failing to link. Mirror OALS's option block here: opt-in OFF by default, forced ON on APPLE (no other transport works), translated to a compile definition.
oannetutils' LowLatSocket calls NetworkMapper::get_mac_by_uid which lives in oancommon — but oancommon already links oannetutils, so the cycle can't be closed at the CMake level. macOS used -undefined,dynamic_lookup to paper over this; Linux defaulted to strict -no-undefined and the .so itself failed to link. Add --unresolved-symbols=ignore-in-shared-libs on Linux to match the macOS behavior. The engine binary always loads both .so's so the symbol resolves correctly at runtime.
The previous flag tells ld to be lenient about undefined symbols in *sibling .so files* we link against — but oannetutils links no other .so, so the flag did nothing and the missing NetworkMapper symbol still failed the build. -z undefs (the Linux equivalent of Mac's -undefined,dynamic_lookup) allows undefined symbols *in this output .so*, deferring them to load-time resolution.
|
On the conceptual side, I am ok with what have been produced. I am less satisfied with the code style though, it is maybe too much java-ish. For instance, when methods are not templated or tied to a header, they must be implemented in the corresponding .cpp file. Sometimes I've seen some C-styled code, when you could've used the C++ syntax (e.g. when a function param might be unused, please use the [[maybe_unused]] attribute). Abstracting LowLatSocket is a good idea, but I'd would more have a static abstraction rather than real inheritance, mostly to avoid the vtables cost. Which is peanuts on powerful system, but is not on embedded systems. Remember: 1 audio stream == 1500 packets per seconds, so every copy, calls, etc... count. I'll make specific comments directly on the lines in the PR. |
|
Notes on AI use and PRs. Code style and clarity should not be neglected. |
Moi78
left a comment
There was a problem hiding this comment.
As I said before, I am overral satisfied on the conceptual part. But the code quality have to be improved. I am not blaming or shaming you, I know that you are not a C++ dev, but there are some improvements to do on that side. Obviously I'll help for that.
|
|
||
| bool RawLinuxTransport::open(const std::string& iface, EthProtocol proto, | ||
| uint16_t self_uid, IfaceMeta& out_meta) { | ||
| (void)self_uid; |
There was a problem hiding this comment.
It is the kind of situation where we want to use [[maybe_unused]] in the parameters of the function.
bool RawLinuxTransport::open(const std::string& iface, EthProtocol proto, [[maybe_unused]] uint16_t self_uid, IfaceMeta& out_meta) { ... }|
|
||
| int RawLinuxTransport::send(const uint8_t* data, size_t len, uint16_t dest_uid) { | ||
| (void)dest_uid; | ||
| return ::sendto( |
There was a problem hiding this comment.
Please, do not use unnamed namespaces
|
|
||
| bool parse_mac(const std::string& s, uint8_t out[6]) { | ||
| unsigned int v[6]; | ||
| if (std::sscanf(s.c_str(), |
There was a problem hiding this comment.
We are using C++20, std::format is preffered
| while (true) { | ||
| // Phase 1: read SimFrame header. | ||
| if (!m_have_hdr) { | ||
| while (m_hdr_read < sizeof(m_hdr_buf)) { |
| } | ||
|
|
||
| // Phase 2: read body. | ||
| while (m_body_read < m_body_expected) { |
There was a problem hiding this comment.
This loop is almost the same as the one before. It could be put in a function.
| # on shared libs, so we relax each library's own link step (the symbol | ||
| # is unresolved in the .so but always resolved at exe link time when | ||
| # both .so's appear on the line — see executables that link them). | ||
| if(APPLE) |
There was a problem hiding this comment.
This should not be necessary. If it is, the architecture of the code must change so that it is not a problem anymore on the linking stage.
| # the parent OALS build; mirror the logic here so OAN is self-buildable | ||
| # (e.g. for CI of OAN-only changes). Forced ON on macOS — there is no | ||
| # other working transport on Apple at the moment. | ||
| option(OAN_HOST_BACKENDS "Build host dev transports + RT shim" OFF) |
There was a problem hiding this comment.
The RT shim is a good idea, I think we should use it for all platforms and leave it on permanently
There was a problem hiding this comment.
Nice addition. We should check for embedded platforms that it still compiles though. e.g. trying to cross-compile the lib with gcc-arm-none-eabi or in a Zephyr context.
There was a problem hiding this comment.
AFAIK we are already using the system to wait for new data, so technically we are not doing busy polling. The timeout is a good idea though.
There was a problem hiding this comment.
Transport backend should be linked compile time. I do agree that the previous impl was not great. But it is too much spaghetti code right now.
No description provided.