fix/audit-perf #23

Merged

faicel merged 3 commits from fix/audit-perf into dev 2026-09-05 22:47:09 +00:00

Conversation 0 Commits 3 Files changed 25 +2346 -1267

faicel commented 2026-09-05 22:45:21 +00:00

Owner

Copy link

No description provided.

faicel added 2 commits 2026-09-05 22:45:21 +00:00

- **Breaking**: migrate to the tools 1.x register API. The legacy ... fb8615b0df

`tools::register::{read_register, write_register}` re-exports no
  longer
  exist: the SX1278 driver now calls
  `tools::register::msb_flag::{read_register, write_register}` and
  propagates the `Result` returned by the fallible register layer. The
  SX1278 SPI wire bytes are unchanged (write command is still
  `address | 0x80`, read command `address & 0x7F`); only import paths
  and
  error handling migrate. Note that the `tools` bus layer now sends a
  write
  as two SPI write operations (command byte, then payload) inside a
  single
  chip-select window; reads use one full-duplex transfer as before.
- **Breaking**: `LoRaError` gains a second generic parameter, the
  chip-select pin error type (`LoRaError<SpiErr, CsErr>`), and two new
  variants: `Cs(CsErr)` for chip-select failures and `RegisterProtocol`
  for register accesses rejected by the `tools` protocol layer. SPI and
  chip-select bus failures are now surfaced instead of being masked.
- **Breaking**: methods that touched registers and were previously
  infallible now return `Result<(), LoRaError<..>>`:
  `set_mode`, `set_frequency`, `deep_sleep`, `wake_from_deep_sleep`;
  `read_version` returns `Result<u8, LoRaError<..>>`.
- Dependency: move to the `tools` 1.x line. `tools` 1.x is not published
  to
  any registry yet, so the crate uses a path dependency (`../tools`) for
  now; the registry line is kept commented with the target version
  (`1.2.1`) to restore once published.
- The SX1278 FIFO is now filled (TX) and drained (RX) in multi-byte
  bursts
  of up to 32 bytes per chip-select window (the MSB-flag protocol frame
  cap) instead of one register access per byte. The FIFO payload bytes
  are
  identical; only the SPI transaction grouping changes. Pure chip-select
  delay per full 255-byte FIFO drops from about 5.1 ms to about 0.16 ms
  in
  each direction. Note that FIFO reads now use the two-transaction burst
  shape (command write, then full-duplex payload) instead of the single
  full-duplex transfer used for one-byte reads.

- Documentation: add a README "Security characteristics" section stating ... 2c1a3f8cf2

what
  the driver does and does not provide: payloads cross the air interface
  without authentication or confidentiality (LoRa PHY plus the chip's
  payload
  CRC, an integrity check rather than a cryptographic measure), the sync
  word
  is not an access control, and message authentication/encryption as
  well as
  regulatory compliance (duty cycle, TX power) remain the application's
  responsibility — the driver enforces the 137-525 MHz band and defaults
  to a
  low-power PA configuration (PA_BOOST, output level 3, normal PA_DAC).
  The
  section also documents that the payload CRC is always enabled and
  cannot be
  disabled through the public API. The Status section's test count is
  refreshed to the current 47 unit tests. Docs only, no code change.

- Documentation: the doc build is now clean without the `sx1278`
  feature.
  Five intra-doc links that referenced feature-gated items were
  converted to
  plain code spans (the crate-root mention of the `sx1278` module and
  the four
  `crate::sx1278::params::…` references in the error model), so `cargo
  doc`
  no longer fails on unresolved links under `--no-default-features`; the
  featureless configuration stays buildable and documentable. Six
  datasheet
  bit-range notations in the payload-CRC doc comments
  (`SymbTimeout[7:0]`,
  `SymbTimeout[9:8]`, `bits [1:0]`) were likewise converted to code
  spans,
  which also restores a clean `--all-features` doc build. Doc comments
  only.

- **Breaking**: remove the never-constructible
  `LoRaError::CompressionFailed`
  variant (dead public API: the compression path falls back to raw
  framing and
  cannot produce this error). Downstream note: exhaustive `LoRaError`
  matches
  must drop the arm.

- Controller initialization no longer silently discards reset-pin
  failures:
  if the reset pin cannot be driven low or high, the failure is logged
  as a
  warning (including the pin error) and initialization continues
  unchanged.
  Reset stays non-fatal by design; the chip-version check remains the
  init
  gate.

- README "Blocking delays" section: documents every fixed blocking-delay
  constant in `params` (name, value, purpose, and the "empirical; origin
  unclear — measure before changing" status), the ~1.3 s initialization
  cost,
  the runtime-configurable RX poll interval (`set_rx_poll_interval_ms`),
  the deep-sleep wake cost of `handle_dio0_irq`, and a GPIO-plus-scope
  measurement recipe for re-validating each value on hardware.
  Docs only, no code change.

- Security fix on the single-receive configuration: the payload-CRC
  enable
  bit of `MODEM_CONFIG_2` is now always forced on instead of being
  derived
  from the RX timeout value — previously, any timeout whose low bits
  cleared
  that bit silently disabled the payload CRC on the air interface
  (corrupted
  packets could be delivered as valid). The symbol timeout is now
  written to
  `REG_SYMB_TIMEOUT_LSB` (0x1F) as a symbol count; the default (100 =
  0x64)
  equals the chip's post-reset register default, so the on-wire default
  behavior is unchanged. `set_rx_timeout` now takes effect on the next
  mode
  restore: re-applying single-receive mode re-enters RX single mode and
  re-arms the symbol timeout. Downstream note: behavior fix, no API
  break.

- Security fix: the TX completion log no longer contains the payload
  bytes.
  The `debug` record emitted after a successful transmission now carries
  metadata only (mode, FIFO byte count, framing description). Rationale:
  a
  radio driver must not ship a payload-egress log path by default —
  payload
  sensitivity belongs to the application layer, which decides what may
  be
  logged and at which level.

- Controller initialization now programs the three consecutive FIFO
  pointer
  registers (address pointer 0x0d, TX base 0x0e, RX base 0x0f) in one
  SPI
  burst write: one chip-select window instead of three, per the
  burst-write
  behavior documented in the SX1276/77/78/79 datasheet (SPI Interface:
  the
  register address auto-increments across data bytes, for both read and
  write accesses). The same three registers are written with the same
  all-zero values; only the write order changes (now ascending by
  address),
  which is unobservable at init with no packet in flight.

- `Controller::set_frequency` now programs the frequency with a single
  SPI
  burst write: the three consecutive FRF registers (0x06-0x08) go out in
  one
  chip-select window instead of three, per the burst-write behavior
  documented in the SX1276/77/78/79 datasheet (SPI Interface: the
  register
  address auto-increments across data bytes, for both read and write
  accesses). The written register bytes are identical, so the programmed
  frequency is unchanged.

- The RSSI and SNR latched packet metrics are now read in a single SPI
  burst
  transaction (one chip-select window instead of two): the burst starts
  at
  the SNR register (0x19) and the address auto-increments to the RSSI
  register (0x1a) inside the same access, per the burst-access behavior
  documented in the SX1276/77/78/79 datasheet (SPI Interface). Register
  values and the RSSI/SNR conversion formulas are unchanged, so reported
  results are identical.

- The LZSS windows moved from the call stack into the `Controller`
  struct:
  `send` and `receive` now use the `tools` `_with_buffer` compression
  functions with one controller-owned window instead of stack-resident
  codec
  windows. Transient stack usage drops by exactly 2048 bytes in `send`
  and
  1024 bytes in `receive` (the 255-byte FIFO payload buffers remain on
  the
  stack in both paths); in exchange the `Controller` struct grows by
  2048
  bytes of persistent RAM, since the single shared window lives in the
  struct. The window is never active in both directions simultaneously
  (both paths run under `&mut self`) and the codec fully re-initializes
  it
  on every call, so sharing is safe. Wire bytes, framing, and register
  transactions are unchanged. Downstream note: `Controller` is larger by
  2048 bytes — relevant for `static`/RTIC resource placement budgets; a
  stack-allocated `Controller` grows its construction frame by the same
  2048 bytes but loses the matching window spike inside `send`. A new
  README "Stack and RAM usage" section documents this accounting.

- The TX payload diagnostic (full FIFO contents logged after every
  successful transmission) moved from the `info` log level to `debug`:
  no
  formatting cost when the `debug` level is filtered off, and the up to
  255-byte payload dump no longer appears on the log stream for users
  running at `info`. The diagnostic remains available at `debug`.

- `Controller::rx_poll_interval_ms()` getter and
  `Controller::set_rx_poll_interval_ms(ms)` setter: the blocking wait
  before
  every IRQ-flags poll in `receive` is now runtime-configurable. Default
  unchanged at 50 ms (identical wire behavior and poll pacing); `0`
  disables
  the pre-poll wait, for IRQ-driven callers that only poll after a DIO0
  interrupt.

- `Controller::set_frequency` now validates the input against the SX1278
  operating band and returns the new `LoRaError::InvalidFrequency` for
  any
  frequency outside 137-525 MHz (bounds inclusive); the check runs
  before any
  register access, so a rejected frequency emits no SPI transactions.
  Note
  for downstream users: exhaustive matches over `LoRaError` must handle
  the
  new variant.

- Remove the stale `deny.toml` advisory suppression for
  `RUSTSEC-2023-0089`
  (heapless 0.7): the suppressed crate chain (`tools` →
  `chacha20poly1305` →
  `heapless`) is no longer in this project's dependency tree, and a
  stale
  global ignore would mask a real occurrence of the advisory if that
  chain
  ever re-enters the tree. The advisory, if it still applies, belongs to
  the
  `tools` repository's own cargo-deny configuration. Config-only change.

- Remove the stale root documents (`revue.md`, `revueOpenIa.md`,
  `TEMPLATE_PUBLICATION_STATUS.md`): untracked working notes whose
  content is
  outdated and contradicted by the current crate; removed at the owner's
  explicit request.

- Documentation: refresh the README to match the current crate —
  dependency
  examples moved to the `0.4` version line, project structure tree
  updated
  (the `params` module and the error tests directory are listed, the
  dead
  crate-root file is gone), the dependency list corrected to the real
  set
  (`embedded-hal`, `log`, `tools` with its `register` and `compression`
  features), the status section updated to version 0.4.0 with a factual
  driver and test summary, the `sx1278` cargo feature documented as the
  real
  gate of the `sx1278` module (the removed `test-support` row is
  dropped),
  and the usage example now handles the `Result` of every fallible call.
  Docs only, no code change.

- Tooling: correct the `deny.toml` header comment, which still
  referenced
  the wrong crate due to copy-paste drift; it now names this crate.
  Comment-only change.
- Manifest: remove the redundant `license` key from `Cargo.toml` — cargo
  warned on every build that only one of `license` or `license-file` is
  necessary. `license-file = "LICENSE"` is kept for the custom
  non-commercial license (cargo-deny resolves it from the LICENSE file
  hash). No code change.
- Tests: remove the unused const-generic parameter from the
  receive-expectation
  test helper (the buffer size was never controlled by it) and drop the
  meaningless turbofish at every call site. Dead parameter only:
  identical
  expectations, identical test count, no driver code change.
- Tests: the TX-timeout test now derives its expected IRQ-flags poll
  count
  from `TxTimeoutConfig::default()` (ceiling division of `timeout_ms` by
  `poll_interval_ms`, mirroring the driver polling loop) instead of a
  hardcoded literal, so changing the default timeout configuration no
  longer
  requires editing the test. Same poll count asserted; no coverage
  change and
  no driver code change.
- Tests: extract the compression-framing mirror duplicated inline in two
  send
  tests (TX timeout and cancel-on-flag) into one shared `framed_fifo`
  helper
  next to the canonical mirror in the common test module; the mocked bus
  expectations of both tests are unchanged. No coverage change and no
  driver
  code change.
- Merge the two duplicated unexpected-IRQ receive tests into one
  correctly
  named and documented test: the mocked IRQ value `0x20` is the
  payload-CRC-error bit (`IRQ_PAYLOAD_CRC_ERROR_MASK`), so the covered
  scenario is the CRC-error bit set without RX_DONE, which clears the
  latched flags and yields `NoPacket` with the buffer untouched. Test
  count
  drops by one; no driver code change.
- Deduplicate the receive tests: three identical no-packet tests (same
  register sequence, same `NoPacket` error, only the buffer filler bytes
  differed) collapse into one test that still asserts the IRQ-flags
  poll,
  the `NoPacket` result, the unchanged buffer, and the exact bus
  transactions. Test count drops by two; no driver code change.

- Documentation: correct the `send` TX_DONE wait description — the
  timeout
  defaults to 500 ms via `TxTimeoutConfig::default()` and is
  configurable,
  not 1 second — and add `Mode::ReceiverSingle` to the accepted-modes
  list
  of `set_mode`. Doc comments only, no code change.

- `Controller::get_mode` and `Controller::get_rx_timeout` now take
  `&self`
  instead of `&mut self` — a source-compatible relaxation for existing
  callers (receiver auto-reborrow), not an API break.
- Name and document the SX1278 blocking delays: every hardcoded
  `delay_ms` literal in the driver (reset sequence, boot, TX/RX entry,
  abort, wake from deep sleep) becomes a one-line-documented constant in
  `params`. Identical numeric values and identical register transactions
  —
  no behavior change.

- Enforce `#![warn(missing_docs)]` at the crate root and complete the
  public
  API documentation it requires: crate-level docs, `Controller`,
  `TxTimeoutConfig` (with its fields), the RX/TX timeout accessors,
  `send`,
  the `registers` constants, and the public test modules. Docs and lint
  attribute only, no behavior change.
- RSSI/SNR value assertion test: a receive test now checks the computed
  metrics from mocked register values (RSSI register 0x5F and SNR
  register
  0x0A must yield -69 dBm and +10 dB), pinning the offset formula
  against
  regressions; previous tests never asserted these numbers.

- Fix the RSSI offset in the receive path: the SX1278 operates the LF
  port
  only (137-525 MHz band, Semtech SX1276/77/78/79 datasheet Rev. 7 - May
  2020, Table 1 p.10 and Table 32 p.82), so the LF-port formula
  RSSI(dBm) = -164 + Rssi applies. The driver previously used the
  HF-port
  formula (-157 + Rssi), a 7 dB error at 433 MHz; reported RSSI values
  shift
  down by 7 dBm (metrics only, protocol behavior unchanged).

- Remove the dead RSSI selection constants `RF_MID_BAND_THRESHOLD` and
  `RSSI_OFFSET_HF_PORT`: the single-band SX1278 needs no per-frequency
  port
  selection, so only the verified LF-port offset remains.

- Unit tests for the bus-error to `LoRaError` conversion boundary: one
  test
  per mapping arm — `Spi` and `Cs` failures forward their typed payload
  unchanged, while the protocol rejections `PayloadTooLong` and
  `InvalidRegister` collapse into `RegisterProtocol` — using two
  distinct
  dummy error types to pin the generic parameters.
- Test coverage for the `Mode::ReceiverSingle` public mode: end-to-end
  tests
  for controller construction, mode switch, and deep-sleep wake,
  asserting
  the exact RX_SINGLE register sequence (op mode write plus
  `MODEM_CONFIG_2`
  read-modify-write re-applying the RX timeout bits).

- Wire the `sx1278` feature gate: builds without the feature no longer
  compile or expose the `sx1278` module; only `lora::error` remains
  public.
  Consumers must enable the feature (both known consumers already
  request
  `sx1278` with `default-features = false`). As a result, bare `cargo
  test`
  without features now runs 0 tests and exits 0; the suite runs with
  `cargo test --features sx1278`, which is what CI already uses.

- Remove the `test-support` feature and the optional `embedded-hal-mock`
  dependency block behind it, together with the public
  `lora::sx1278::tests`
  helper exports (`BusExpectations` and the mock expectation builders)
  that
  the feature enabled: no consumer exists — verified against both known
  consumers. `embedded-hal-mock` remains a plain dev-dependency for the
  crate's own unit tests.
- Delete the dead `src/mod.rs` crate-root file: it contained only
  `pub mod registers;` (pointing at a non-existent `src/registers.rs`)
  and was
  referenced by nothing — the crate root is `src/lib.rs`, so the file
  never
  compiled.
- Remove the unreachable duplicate CRC-error branch in the SX1278
  receive
  path: an identical earlier check on the same IRQ flags already returns
  the
  error, so the second branch could never execute. No behavior change.
- Remove the unused public constant
  `registers::constants::MAX_PKT_LENGTH`
  (it duplicated `params::MAX_LORA_FIFO_BYTES` and was referenced
  nowhere)
  together with the now-orphaned import of that constant, and delete all
  commented-out register constants and commented-out configuration
  alternatives (they are preserved in git history). No code change.

faicel added 1 commit 2026-09-05 22:45:31 +00:00

Merge branch 'dev' into fix/audit-perf e40f05dea9

faicel merged commit 1dcc3f2c3f into dev 2026-09-05 22:47:09 +00:00

faicel deleted branch fix/audit-perf 2026-09-05 22:47:09 +00:00

faicel referenced this pull request from a commit 2026-09-05 22:47:10 +00:00

Merge pull request 'fix/audit-perf' (#23) from fix/audit-perf into dev

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

homeiot/lora!23

No description provided.