Add I2C support to RP2 platform

[pguyot]

Continuation of #2111

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

[petermm]

per https://ampcode.com/threads/T-019cfaf6-743c-757d-815f-f4a647d40079

usual caveats

PR Review: Add I2C support to RP2 platform

Commit: 53ff0bb — Add I2C support to RP2 platform
Files changed: 9 files, +1112 lines

---

Overview

Adds I2C support for the Raspberry Pi Pico (RP2) platform with:

• i2cdriver.c — C NIF driver wrapping Pico SDK I2C functions
• i2c.erl — Erlang module with low-level NIFs and high-level process-based API
• gpio:set_function/2 — new NIF for pin muxing
• Two example programs (pico_i2c_scanner, pico_lis3dh)
• CMake build integration

Binary/GC handling is correct — no use-after-GC bugs found. Resource rooting via memory_ensure_free_with_roots and read-path MEMORY_NO_GC pre-allocation are done properly.

---

Must-Fix Issues

1. Use-after-deinit crash (i2cdriver.c)

nif_i2c_deinit sets rsrc_obj->i2c_inst = NULL, but no other NIF checks for NULL before calling Pico SDK functions. This causes a crash on:

• Double deinit(Resource)
• Any operation after deinit(Resource)

Fix: Check rsrc_obj->i2c_inst at the top of every NIF:

if (UNLIKELY(IS_NULL_PTR(rsrc_obj->i2c_inst))) {
    RAISE_ERROR(BADARG_ATOM);
}

Make deinit/1 idempotent (return ok if already closed).

---

2. Duplicate peripheral ownership (i2cdriver.c:88–131)

i2c_get_instance(peripheral) returns one of two global hardware instances, but nif_i2c_init allocates a fresh resource every time. Calling init(0, ...) twice creates two resources for the same physical controller:

• Closing one deinitializes the controller for the other
• Baudrate changes through one affect the other
• Destructor on either resource can pull hardware out from under the other

Fix: Track per-peripheral ownership:

static bool i2c_in_use[NUM_I2C_INSTANCES] = {false, false};

Return {error, busy} if already open. Clear the slot on deinit/dtor.

---

3. term_to_nostop does not validate booleans (i2cdriver.c:80–86)

static bool term_to_nostop(term t)
{
    if (UNLIKELY(!term_is_atom(t))) {
        return false;   // non-atom silently becomes false
    }
    return t == TRUE_ATOM;  // any atom except 'true' becomes false
}

Passing banana as the nostop argument silently succeeds as false.

Fix: Strict boolean validation:

static bool term_to_bool(term t, bool *out)
{
    if (t == TRUE_ATOM) { *out = true; return true; }
    if (t == FALSE_ATOM) { *out = false; return true; }
    return false;
}

Then RAISE_ERROR(BADARG_ATOM) on failure at call sites.

---

4. Missing range checks on integer arguments (i2cdriver.c)

Several integer arguments are only checked with term_is_integer then cast:

Argument	Current	Should be
addr	cast to uint8_t	0 ≤ addr ≤ 127 (7-bit I2C)
baudrate	cast to uint	> 0
timeout_us	cast to uint	≥ 0, fits in uint
count	checked ≥ 0	also cap to a sane max

Negative values wrap silently via unsigned cast.

---

5. Erlang server crash = caller hangs forever (i2c.erl)

call/2 sends a message and waits with no monitor and no timeout:

call(Pid, Request) ->
    Ref = make_ref(),
    Pid ! {self(), Ref, Request},
    receive
        {Ref, Reply} -> Reply
    end.

If the server process dies (e.g., bad write_byte value crashes <<Byte:8>> construction), callers block forever.

Fix: Monitor the server:

call(Pid, Request) ->
    MRef = monitor(process, Pid),
    Ref = make_ref(),
    Pid ! {self(), Ref, Request},
    receive
        {Ref, Reply} ->
            demonitor(MRef, [flush]),
            Reply;
        {'DOWN', MRef, process, Pid, Reason} ->
            {error, Reason}
    end.

Also add input validation in handle_request/4 and a catch-all clause returning {error, badarg}.

---

Should-Fix Issues

6. Direct ops allowed during buffered transaction (i2c.erl:428–450)

This is currently permitted but surprising:

begin_transmission(I2C, Addr),
write_byte(I2C, 16#00),
read_bytes(I2C, Addr, 4),    %% executes immediately — intended?
end_transmission(I2C).

Fix: When TxState =/= undefined, reject read_bytes/write_bytes with {error, transaction_in_progress}. Only allow write_byte, write_bytes (tx variant), end_transmission, and close.

---

7. open/1 missing parameter validation (i2c.erl:116–127)

• Missing {scl, _} or {sda, _} → proplists:get_value returns undefined → crash in gpio:set_function/2
• No check that SCL ≠ SDA
• No check that pins are compatible with the selected peripheral
• If pin muxing succeeds but init/2 fails, pins are left reconfigured with no rollback

Fix: Validate before side effects:

open(Params) ->
    SCL = proplists:get_value(scl, Params),
    SDA = proplists:get_value(sda, Params),
    (is_integer(SCL) andalso is_integer(SDA) andalso SCL =/= SDA)
        orelse error(badarg),
    ...

---

8. Blocking NIFs risk stalling the VM

The high-level API wraps blocking NIFs. With send_timeout_ms = infinity (or large timeouts), a wedged I2C bus can stall the entire scheduler.

Recommendation: Default to finite timeouts in the high-level API and document that infinity may block the VM.

---

What Looks Good

• GC / binary handling — memory_ensure_free_with_roots correctly roots the resource term; read paths pre-allocate with MEMORY_NO_GC; write paths use term_binary_data immediately without intervening allocations
• Resource destructor — i2c_resource_dtor checks for NULL before calling i2c_deinit
• gpio:set_function/2 — clean implementation with proper pin range and atom table validation
• API design — dual low-level/high-level API is well structured and consistent with the existing ESP32 i2c_hal behavior pattern
• Examples — clear, practical, well-documented
• Documentation — thorough edoc with type specs throughout i2c.erl

---

Suggested Test Cases

Before merge, these scenarios should be covered:

• deinit(Resource) called twice → ok (idempotent)
• Operation after deinit(Resource) → badarg or {error, closed}
• init(0, ...) called twice → {error, busy}
• write_blocking(Resource, Addr, Data, not_a_boolean) → badarg
• Invalid address: -1, 128, 300 → badarg
• Invalid timeout: -1 → badarg
• write_byte(Pid, 300) → error, not server crash
• read_bytes(Pid, Addr, -1) → error, not server crash
• Call to dead server pid → {error, Reason}, not hang
• read_bytes during active begin_transmission → {error, transaction_in_progress}
• open([]) (missing scl/sda) → clean error