Bluetooth: Controller: Periodic Advertising Chan Map Update Indication

[cvinayak]

Added implementation to perform Periodic Advertising Channel
Map Update Indication as a Broadcaster.

Added implementation in Periodic Advertising Synchronization
to support Channel Map Update Indications present in the
ACAD fields of the AUX_SYNC_IND PDUs.

Signed-off-by: Vinayak Kariappa Chettimada vich@nordicsemi.no

[ppryga-nordic]

Request to add missing information in header of function ull_adv_sync_pdu_set_clear.
Other things may be followed up in separate PR (but still provide your feedback).

[ppryga-nordic]

There are multiple places in the controller where traverse by extended advertising header is done. If the structure of the extended advertising header is changed it will be difficult to find all of them to update. It would be good to have common implementation for that.
Lack of check in this place if cte_info is available was a source of very strange error in evaluation of radio wakeup time.
Common implementation should save us from such errors in future.

[cvinayak]

There are multiple places in the controller where traverse by extended advertising header is done.

Not all of the fields are needed to be traversed in the code where the traversal is repeated in the controller implementation. CTE info flag was added in BT v5.1 which was not present in BT v5.0, and the flag was added before ADI and Aux Ptr fields. There are two places the traversal of flags in received PDU is used, in lll_scan_aux.c and ull_scan_aux.c, both have different requirement of handling the fields, a common implementation will only increase complexity of such an interface.

As there is no explicit ACAD len in the PDU, it is not possible to validate based on the PDU length.

What can be done, check if reserved flag is set then regard the complete PDU as invalid. Future features will still need update to all places where traversal is present, which is what we are doing now with CTE field (between v5.0 and v5.1).

[ppryga-nordic]

Well my point is to get "golden mean" between copying the code and re-use of code. E.g. if there is a number of places we get aux_ptr then it should be implemented once and re-used. If there is a need to get access to ACAD, that is a bit more complicated than just traversal ext. header, in multiple places it should also be implemented once.

Having single implementation would save time for updating.
Also when the code is a function that has single responsibility it may be unit tested. Then we are sure if updates does not break already used functionality.

[Thalley]

Could this be moved to a boolean function?

e,g,

Suggested change

	/* At or past the instant, use channelMapNew */
	instant_latency = (event_counter - lll->chm_instant) &
	UINT16_MAX;
	if (instant_latency <= INT16_MAX) {
	static bool is_past_instant(uint16_t event_counter, uint16_t chm_instant)
	{
	uint16_t instant_latency;
	instant_latency = event_counter - chm_instant
	return instant_latency >= INT16_MAX;
	}
	...
	if (is_past_instant(event_counter, lll->chm_instant) {
	...
	}

As this specific calculation is done multiple places

[Thalley]

All my comments have been addressed.