[mclueppers]

Problem

An AnnounceHandler receives the destination hash, the announced identity and the app data, but nothing about how the announce arrived. Applications that keep a neighbour list or show link quality (which interface heard the node, how many hops away it is, its RSSI and SNR) cannot get that from the callback. Today the only route is to also hook the raw receive callback, then parse and verify every announce a second time to match it up, which duplicates Transport's work and doubles the signature checks on small MCUs. Requested in #84.

Solution

Add a second virtual overload, received_announce(destination_hash, announced_identity, app_data, packet), and have Transport call it. Its default implementation forwards to the existing three-argument callback, so every existing handler keeps working unchanged. Handlers that want the metadata override the new overload as well.

The Packet reference is only valid for the duration of the call. This is documented in the header and the README.

Compatibility

• No change to wire behavior vs. the Python reference implementation
• Any divergence from the reference is behind an RNS_* preprocessor guard (off by default): no divergence; this is an API addition
• No breaking public API change: additive overload with a forwarding default; CHANGELOG entry added
• No board- or variant-specific code

Testing

Platforms tested (and how):

• native: pio test -e native, native14, native17, native20 pass 193/193 each, including the new test_rns_loopback case
• ESP32: LilyGO T-Beam (SX1276), running as a transport node; the firmware also builds for Heltec Wireless Stick V2 and Wireless Bridge
• ESP32-S3: RetiMesh Node firmware builds for 10 S3 boards; the change has run on S3 nodes since August
• nRF52: no hardware
• Interop with Python RNS (test_interop/): not run; no wire change

Platforms not tested, and why: nRF52 (no hardware). This repo's embedded PlatformIO environments were not built locally and are left to CI.

Docs

• README, header comment and CHANGELOG updated

Closes #84.

[attermann]

Thanks for this, and for the clear write-up in #84. The default-forwarding approach keeps existing handlers working, so the design looks right to me. A few things are needed per the contribution policy (PR_POLICY.md) before it can merge:

1. Add a small case to test_rns_loopback. It should show that a handler overriding the four-argument form receives the packet, and that a three-argument-only handler is still called.
2. List the platforms you tested and how (§5.5). This touches the shared announce dispatch path.
3. Add a CHANGELOG entry for the new overload (§5.7).
4. In the header comment and the README, note that the Packet is only valid for the duration of the callback, so handlers must copy anything they keep.
5. Say briefly in the description what problem this solves (§5.3). Linking Feature request: give AnnounceHandler access to the announce packet (interface, hops, RSSI/SNR) #84 is fine if that covers it.

CI didn't start on this PR, and I'll deal with that on my side.

[mclueppers]

Thanks for the review. All five points are addressed on the branch, and master is merged in without conflicts.

1. Loopback test. One finding first: the announce in test_rns_loopback is for a local destination, so Transport discards it before the handlers ("Packet is announce for local destination, not processing"). The existing ExampleAnnounceHandler never fires either. The new case captures the announce the test sends, deregisters the destination and replays the packet, so it arrives the way one from another node would. It checks that a handler overriding only the three-argument form is called once, and that a handler overriding the packet-carrying form receives the packet (destination hash, packet type, receiving interface) and is not also called through the basic form. Against master's Transport the packet-handler assertion fails (Expected 1 Was 0). On the branch it passes.
2. Platforms tested.
   • native, native14, native17 and native20 with PlatformIO: 193/193 tests pass on each. native re-run against current microStore master (d5282e6): still 193/193.
   • ESP32-S3 and ESP32: the RetiMesh Node firmware builds on this branch for 13 boards (ESP32-S3: LilyGO T3-S3 incl. SX1280 variants, T-Beam Supreme, T-Deck, Heltec V3/V4/Wireless Paper, ThinkNode M9, generic S3 dev kit; ESP32: LilyGO T-Beam, Heltec Wireless Stick V2, Heltec Wireless Bridge) and uses the four-argument form for its neighbour list.
   • Hardware: a LilyGO T-Beam (ESP32 + SX1276) running as a transport node on this exact branch. Earlier versions of the change have run on our ESP32-S3 nodes since August.
   • Not tested: nRF52 (no hardware), and the embedded environments in this repo's platformio.ini, which I left to CI.
3. CHANGELOG. There is an entry under a new "Unreleased" heading.
4. Packet lifetime. The header comment and the README now say the Packet is only valid for the duration of the callback and that handlers must copy what they keep. The README also no longer says "override either callback": the three-argument form is pure virtual, so it must always be implemented.
5. Problem statement. I've updated the PR description (below), and it closes Feature request: give AnnounceHandler access to the announce packet (interface, hops, RSSI/SNR) #84.

[attermann]

Thanks for working through all of that so carefully. The test catching the local-destination case was a good find: it shows the existing announce handler test was never actually being exercised.

One thing I'll look at before merging: Python RNS can pass the announce packet hash to the handler as an optional extra argument, while this passes the whole Packet. That isn't a blocker, since the Packet gives more, but I want to think about whether we should expose the hash too, for parity with the reference API. I'll do that on my side as time allows.