Also enable AODV route discovery on source-routed unicasts

[TheJulianJES]

DRAFT/TODO: Check this and tidy up AI comment.

I've been somewhat accidentally using this for years by turning on source routing in the zigpy config once, then disabling it. This still caused the coordinator to set source routes, but apparently it never refreshed the network addresses this way.

But only with this was source routing actually worth using in my network: as zigpy thought source routing to be off, it still set APS_OPTION_ENABLE_ROUTE_DISCOVERY as well.

According to some sources referencing "BUGZID 12261", this is supposedly incorrect. This should be looked at with more detail.

AI summary (check if accurate)

When CONF_SOURCE_ROUTING is enabled, bellows previously set only APS_OPTION_ENABLE_ADDRESS_DISCOVERY on outgoing NWK unicasts. The two flags are independent and address different stack subsystems:

• ENABLE_ROUTE_DISCOVERY (NWK frame flag) — initiates AODV route discovery if and only if the stack has no known route to the destination (per sl_zigbee_types.h: "causes a route discovery to be initiated if no route to the destination is known"). Contrast with FORCE_ROUTE_DISCOVERY which initiates discovery unconditionally.
• ENABLE_ADDRESS_DISCOVERY (APS frame flag) — sends a ZDO NWK_addr_req to resolve the destination NWK ID from its EUI64 if not already cached. It does not discover routes; it resolves addresses.

When source routing is on but a destination isn't yet in the NCP's source-route table (e.g. immediately after startup, before MTORR has propagated, or after a device rejoin), the previous configuration left the stack with neither a source route nor permission to fall back to AODV. The packet was dropped at the NWK layer instead of recovering via on-demand route discovery.

This change makes source-routed unicasts also set ENABLE_ROUTE_DISCOVERY, mirroring what zigbee-herdsman's ember adapter does (emberAdapter.ts DEFAULT_APS_OPTIONS):

DEFAULT_APS_OPTIONS = RETRY | ENABLE_ROUTE_DISCOVERY | ENABLE_ADDRESS_DISCOVERY;
// "Removing ENABLE_ROUTE_DISCOVERY leads to devices that won't reconnect/go
//  offline, and various other issues."

Why this is correct (source routing is still preferred)

ENABLE_ROUTE_DISCOVERY does not run alongside source routing on every send. The SDK header text is explicit: discovery happens "if no route is known." The stack's outbound path for a concentrator is:

1. sl_zigbee_af_override_append_source_route_cb runs first. If a source route exists for the destination, it's prepended to the NWK header and each hop along the way uses that explicit route. AODV is not consulted.
2. If no source route exists, the NWK route table is checked. If a previously discovered AODV route is there, it's used.
3. Only if both tables come up empty does the ENABLE_ROUTE_DISCOVERY flag actually trigger an AODV route request.

Net effect: source routing remains the primary path; AODV is engaged strictly as a fallback for destinations the source-route table doesn't yet know about.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.54%. Comparing base (4b97a6d) to head (4d56aed).

Additional details and impacted files

@@           Coverage Diff           @@
##              dev     #719   +/-   ##
=======================================
  Coverage   99.54%   99.54%           
=======================================
  Files          61       61           
  Lines        4147     4147           
=======================================
  Hits         4128     4128           
  Misses         19       19           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[TheJulianJES]

And/or we should consider not ignoring the TransmitOptions.FORCE_ROUTE_DISCOVERY zigpy option when source routing is enabled, like we do now:

bellows/bellows/zigbee/application.py

Lines 993 to 1001 in 4b97a6d

	if self.config[zigpy.config.CONF_SOURCE_ROUTING]:
	# Source routing uses address discovery to discover routes
	aps_frame.options |= t.EmberApsOption.APS_OPTION_ENABLE_ADDRESS_DISCOVERY
	elif zigpy.types.TransmitOptions.FORCE_ROUTE_DISCOVERY in packet.tx_options:
	# Forcing route discovery requires retrying
	aps_frame.options |= t.EmberApsOption.APS_OPTION_FORCE_ROUTE_DISCOVERY
	aps_frame.options |= t.EmberApsOption.APS_OPTION_RETRY
	else:
	aps_frame.options |= t.EmberApsOption.APS_OPTION_ENABLE_ROUTE_DISCOVERY

[TheJulianJES]

TODO: Check if the stack removes broken/outdated source routes, or just keeps using them until the next MTOR happens.

[MattWestb]

Little more experience / problem with routing in ZHA / Bellows.
ZHA was working great then IKEA was using group binding for remotes and motion senors (broadcast commands).
After redoing binding i my production system to unicast / device binding i have 2 problem and i think one is one neighbor using microwave owen and blocking the RF and the next is commands from controllers is not working for short or longtime. Normally after one light have rebooted and getting one new MWK (after the network have refusing using the old one that is 99% one Silabs bug).
Like in my WC with 1 IKEA motion sensor and one dimmer switch sending unicast bond commands to 3 X GU10, Recent spot and one Äskvöder for steering the fan controller.
Symptom is some command not finding its device, Controllers is bonding devices with IEEE and then the controller shall getting the updated / current NWK from its parent and sending unicast to all NWKs but the routers / mesh is not handling it OK so so not all commands is going thru from the motion sensor.
The dimmer switch with the same bindings as the motion sensor is getting different patterns then is getting different reponce from its parent / mesh so you is on the right way !!

Next is 1 tuya 4 way switch that oft is not responding on commands (Zigbee transmission failed) from automatons and is working OK after repower it. Was sniffing it last week and (Z)HA was not sending the on commands that automation was set to do also the sending one command from device card = ZHA is not having one know rout to the device and is not trying sending one command only error in the log.

The system is for very long time running with source_routing: true but the problem have being larger with the time.

By the way: forcing manual source routers is one very danger thing then its disabling the self healing functionality in the mesh network so if one router is getting problem then some device can being not reatshe ball and cant being fixed in one easy way what i knowing so if implanting it pleas put strong warnings in the instructions !!!

Great work done !!!!

[MattWestb]

PS: Coordinator hard and software = Connected via [IKEA Billy EZSP 6.10.7.0]

[zigpy-review-bot]

Reviewed as a draft. The code change is correct and matches zigbee-herdsman's long-standing default — confirming the explanatory mechanism, not just the safety of the change.

Behavior

Verified against the Silicon Labs SDK semantics for APS_OPTION_ENABLE_ROUTE_DISCOVERY: "causes a route discovery to be initiated if no route to the destination is known." So when a source route is present the stack uses it; AODV only kicks in when the source-route table is empty for that destination (immediately after startup before the first MTORR propagates, after a device rejoin where the route record hasn't been re-sent, etc.). Source routing remains the primary path; AODV is strictly a fallback. The new code comment captures this accurately.

zigbee-herdsman's ember adapter sets exactly the same combination in DEFAULT_APS_OPTIONS = RETRY | ENABLE_ROUTE_DISCOVERY | ENABLE_ADDRESS_DISCOVERY and explicitly cites BUGZID 12261 in the same comment block, noting they're ignoring the SDK guidance because removing route discovery causes "devices that won't reconnect/go offline, and various other issues." (See src/adapter/ember/adapter/emberAdapter.ts, DEFAULT_APS_OPTIONS.) This PR brings bellows into alignment with that empirically validated default. The two targeted tests in this PR (test_send_packet_unicast_source_route and test_send_packet_unicast_manual_source_route) both pass locally.

Concerns with the PR description (vs the code)

The PR body and the inline comment in the diff tell slightly different stories — the body is where the tidy-up TODO is needed:

1. "never refreshed the network addresses" mechanism is mis-stated in the body, but corrected in the AI summary. Route discovery doesn't refresh NWK addresses — NWK address mapping for a known EUI64 is refreshed by ZDO Device_annce (on rejoin) or by NWK_addr_req, which is what APS_OPTION_ENABLE_ADDRESS_DISCOVERY (already set) triggers. ENABLE_ROUTE_DISCOVERY discovers a path, not a new NWK ID. The "AI summary" section of the PR body has this right; the prose at the top conflates the two. Worth tightening before merge so the commit message doesn't ship the confused version.

2. BUGZID 12261 isn't refuted, it's overridden by empirical evidence. SDK guidance is genuinely "concentrators should not enable route discovery because MTORRs do it." The justification for ignoring it is the same one zigbee-herdsman documents: stale source-route entries after rejoins / cold start lead to silent packet drops at the NWK layer when AODV isn't allowed to recover. Worth saying in the description that this PR is deliberately diverging from SDK guidance in favor of zigbee-herdsman's tested behavior, not claiming BUGZID 12261 is wrong. See Koenkk/zigbee2mqtt#29902 for the long-form discussion of this tension on the zigbee2mqtt side.

3. Author's "accidental usage for years" history isn't a clean A/B test. The narrative is: source routing was enabled in zigpy config once, then disabled, but the coordinator kept setting source routes. That doesn't actually demonstrate that the combined flags worked — it demonstrates that the source-route table on the NCP persists, and that CONF_SOURCE_ROUTING=False was used (so ENABLE_ROUTE_DISCOVERY was set via the else branch at line 1008, not via the new combined branch). The new combined branch is genuinely a new code path the author hasn't been running. Not a blocker — zigbee-herdsman parity is the stronger argument — but the description shouldn't oversell prior validation.

Minor

• Negative test (CONF_SOURCE_ROUTING=False → ENABLE_ROUTE_DISCOVERY set via the else branch but ENABLE_ADDRESS_DISCOVERY not set) is already implicitly covered by the non-source-routing test_send_packet_unicast. No new negative test needed.
• This complements PRs #716/#717/#718; consider noting the series in the description so a reviewer can see the broader source-routing-hardening context.

Verdict: code change LGTM. Recommend tidying the PR body before un-drafting — particularly the "refreshed network addresses" mechanism and the framing of BUGZID 12261 (diverging from SDK by design, not refuting it).