nimble/ll: Refactor transitions #1974
Conversation
mkasenberg
force-pushed
the
transition
branch
from
ef8591c to
af12eb5
Compare
mkasenberg
force-pushed
the
transition
branch
from
af12eb5 to
679f8b4
Compare
mkasenberg
force-pushed
the
transition
branch
from
679f8b4 to
b4af225
Compare
| void ble_phy_tifs_set(uint16_t tifs); | ||
| #endif | ||
| /* Set T_ifs for the next transition */ | ||
| void ble_phy_tifs_set(uint16_t usecs, uint8_t anchor); |
There was a problem hiding this comment.
this is no longer needed (?)
nimble/drivers/nrf5x/src/ble_phy.c
Outdated
| void | ||
| ble_phy_set_rxend_cb(ble_phy_rx_end_func rxend_cb, void *arg) | ||
| { | ||
| /* Set transmit end callback and arg */ |
There was a problem hiding this comment.
'receive'
what is the purpose of this callback? it doesn't seem to be used anywhere
| * non-zero for end of PDU */ | ||
| void ble_phy_tifs_txtx_set(uint16_t usecs, uint8_t anchor); | ||
| /* Set direction of the next transition */ | ||
| void ble_phy_transition_set(uint8_t trans, uint8_t anchor, uint16_t usecs); |
There was a problem hiding this comment.
I wonder if we should perhaps merge trans and anchor to a single parameter
standard tifs transitions would be called as they are now, but add 2 transitions that anchor at the beginning of pdu, smth like BLE_PHY_TRANSITION_TO_TX_SUB -> sub is short of 'subevent' as this is where it will be used
@sjanc ^^
There was a problem hiding this comment.
yeah, but I'd call it subevent for clarity
nimble/drivers/nrf5x/src/ble_phy.c
Outdated
| } | ||
|
|
||
| void | ||
| ble_phy_fast_transition_enable(void) |
There was a problem hiding this comment.
what is fast transition?
mkasenberg
force-pushed
the
transition
branch
3 times, most recently
from
d5999ac to
7b860c6
Compare
|
#AutoPTS run mynewt nrf52 GATT/CL/GAW/BV-01-C GATT/CL/GAD/BV-01-C GATT/CL/GAR/BV-01-C GATT/CL/GAN/BV-01-C GATT/SR/GAN/BV-01-C |
|
Scheduled PR #1974 (comment), board: nrf52, estimated start time: 08:02:23, test case count: 5, estimated duration: 0:12:34 Test cases to be runGATT/CL/GAD/BV-01-CGATT/CL/GAR/BV-01-C GATT/CL/GAW/BV-01-C GATT/CL/GAN/BV-01-C GATT/SR/GAN/BV-01-C |
|
AutoPTS Bot results: Failed tests (1)GATT GATT/SR/GAN/BV-01-C INDCSVSuccessful tests (4)GATT GATT/CL/GAD/BV-01-C PASSGATT GATT/CL/GAN/BV-01-C PASS GATT GATT/CL/GAR/BV-01-C PASS GATT GATT/CL/GAW/BV-01-C PASS |
| NRF_TIMER0->EVENTS_COMPARE[2] = 0; | ||
| phy_fem_enable_pa(); | ||
| #endif | ||
| transition = g_ble_phy_data.phy_transition; |
There was a problem hiding this comment.
txend_cb will setup next transition so you need to read all parameters for current transition before that.
nimble/drivers/nrf5x/src/ble_phy.c
Outdated
|
|
||
| /* Return to default values for the next transition */ | ||
| g_ble_phy_data.phy_transition = PHY_TRANS_NONE; | ||
| g_ble_phy_data.tifs_anchor = PHY_TRANS_ANCHOR_END; |
There was a problem hiding this comment.
you cannot reset parameters here because it will overwrite transition that was set in txend_cb.
if you want to reset to default parameters just in case code doesn't set transition, this should be done after reading parameters for current transition and before calling txend_cb.
| /* Packet pointer needs to be reset. */ | ||
| ble_phy_rx_xcvr_setup(); | ||
| next_phy_mode = g_ble_phy_data.phy_rx_phy_mode; | ||
| } |
There was a problem hiding this comment.
this should be done as part of transition_to_rx code
nimble/drivers/nrf5x/src/ble_phy.c
Outdated
|
|
||
| ble_phy_transition_fem(transition, start_time); | ||
|
|
||
| if (transition == PHY_TRANS_TO_TX) { |
There was a problem hiding this comment.
I'd split this into smaller blocks, i.e.
if (transition == PHY_TRANS_TO_TX) {
ble_transition_to_tx(...);
} else if (transition == PHY_TRANS_TO_RX) {
ble_transition_to_rx(...);
} else {
ble_transition_to_none(...);
}
the ble_transition_to_... functions could combine also FEM code so there's no multiple ifs for the same conditions
mkasenberg
force-pushed
the
transition
branch
from
7b860c6 to
e772342
Compare
New Bluetooth features require fast RX-to-RX and TX-to-TX transitions and use a different TIFS than 150us. Co-authored-by: Andrzej Kaczmarek <[email protected]>
New Bluetooth features require fast RX-to-RX and TX-to-TX transitions and use a different TIFS than 150us.