========================== PHY BASED BTS ========================== Valid for: osmo-bts-litecell15 osmo-bts-oc2g osmo-bts-sysmo osmo-bts-octphy l1_if.c:static handle_ph_data_ind() ->static process_meas_res() (Fill up l1sap.u.info.u.meas_ind) -> l1sap_up() (for TCH call tch.c:l1if_tch_rx() and return) (fill up l1sap->u.data) -> l1sap_up() Valid for: osmo-bts-litecell15 osmo-bts-oc2g osmo-bts-sysmo osmo-bts-octphy* tch.c:l1if_tch_rx() ->common/l1sap.c:add_l1sap_header()->l1sap_up(); (sends tch ind.) (does not add another meas_ind as this is already done) *l1if_tch_rx() is in l1_tch.c Proposed changes: The early call to process_meas_res is removed from handle_ph_data_ind. The information added late in l1if_tch_rx() and at the bottom of handle_ph_data_ind() ==> measurement info in all data and tch indications that are handed over to the common code ========================== TRX BASED BTS ========================== Valid for: osmo-bts-trx scheduler_trx.c:tx_data_fn() -> l1_if.c->l1if_process_meas_res()->l1sap_up() (sends meas_ind) common/scheduler.c:_sched_compose_ph_data_ind()->l1sap_up() (sends data) (directly one after another in the middle) (!) Why do we use hardcoded values here? (!) Why do we use hardcoded frame number for data (0) and the real fn for meas? scheduler_trx.c:rx_data_fn() -> l1_if.c->l1if_process_meas_res()->l1sap_up() (sends meas_ind) common/scheduler.c:_sched_compose_ph_data_ind()->l1sap_up() (sends data) (directly one after another at the end) (!) 4 * (*toa256_sum) / *toa_num != *toa256_sum / *toa_num -- Why that? (!) Cosmetic: link_qual_cb should be NULL instead of 0 scheduler_trx.c:rx_pdtch_fn() -> l1_if.c->l1if_process_meas_res()->l1sap_up() (sends meas_ind) (may return 0 when PDTCH is bad) common/scheduler.c:_sched_compose_ph_data_ind()->l1sap_up() (sends data) (in case of bad PDTCH we will not get any measurement info anymore or we allow to hand up bad pdtch data --> problem?) (!) 4 * (*toa256_sum) / *toa_num != *toa256_sum / *toa_num -- Why that? (!) fn != (fn + GSM_HYPERFRAME - 3) % GSM_HYPERFRAME scheduler_trx.c:rx_tchf_fn() -> l1_if.c->l1if_process_meas_res()->l1sap_up() (sends meas_ind) (complex decision logic in between, decides between TCH and FACCH) common/scheduler.c:_sched_compose_ph_data_ind()->l1sap_up() (sends facch) common/scheduler.c:_sched_compose_tch_ind()->l1sap_up() (sends tch, located at the end) (!) FACCH: tn != (fn + GSM_HYPERFRAME - 7) % GSM_HYPERFRAME ? (!) FACCH: toa256 != 4 * toa256 (!) FACCH: Cosmetic: link_qual_cb should be NULL instead of 0 (!) TCH: fn != (fn + GSM_HYPERFRAME - 7) % GSM_HYPERFRAME ==> Extend _sched_compose_tch_ind() so that it also takes the missing measurement related parameters. scheduler_trx.c:rx_tchh_fn() (same as with rx_tchf_fn()) (!) FACCH: *first_fn != fn + GSM_HYPERFRAME - 10 - ((fn % 26) >= 19)) % GSM_HYPERFRAME? (!) FACCH: toa256 != toa256/64 (!) FACCH: Cosmetic: link_qual_cb should be NULL instead of 0 (!) TCH: *first_fn != fn + GSM_HYPERFRAME - 10 - ((fn%26)==19) - ((fn%26)==20)) % GSM_HYPERFRAME ==> Merging MEAS with DATA/TCH is possible, but to get the measurement trough we will likely have to send empty data up along with the measurement. Call to l1if_process_meas_res() is removed, data is edded in the _sched_compose_...() functions. Valid for: osmo-bts-virtual l1_if.c:virt_um_rcv_cb() only when GSMTAP_CHANNEL_PTCCH: -> l1if_process_meas_res->l1sap_up() (sends meas_ind) ==> The function virt_um_rcv_cb() seems to be the only one that triggers the sending of measement indications. When meas_ind is sent also data is sent, so the two can be merged in one. ========================== COMMON ========================== l1sap.c:l1sap_mph_info_ind(): ->l1sap_info_meas_ind(): ->measurement.c:lchan_meas_process_measurement() ==> Remove call to l1sap_info_meas_ind(), log an error instead. This should catch all errornously upcomming measurement indications. Maybe have an OSMO_ASSERT(false) there in the beginning. l1sap.c:l1sap_up() ->l1sap_ph_data_ind() ->l1sap_tch_ind() ==> The functions l1sap_ph_data_ind() and l1sap_tch_ind() can take over the role of l1sap_info_meas_ind() and take the measurement related data from the tch and data indications to forward them into to lchan_meas_process_measurement() ========================== LIBOSMOCORE ========================== l1sap.h: This is what a meas_ind currently transfers: struct info_meas_ind_param { uint8_t chan_nr; /*!< Channel Number (Like RSL) */ uint32_t fn; /*!< GSM Frame Number */ uint16_t ber10k; /*!< BER in units of 0.01% */ union { int16_t ta_offs_qbits; /*!< timing advance offset (in qbits) */ int16_t ta_offs_256bits;/*!< timing advance offset (in 1/256th bits) */ }; int16_t c_i_cb; /*!< C/I ratio in 0.1 dB */ uint8_t is_sub:1; /*!< flags */ uint8_t inv_rssi; /*!< RSSI in dBm * -1 */ }; This is what data and tch indications already transfer struct ph_data_param { uint8_t link_id; /*!< Link Identifier (Like RSL) */ uint8_t chan_nr; /*!< Channel Number (Like RSL) */ uint32_t fn; /*!< GSM Frame Number */ int8_t rssi; /*!< RSSI of receivedindication */ uint16_t ber10k; /*!< BER in units of 0.01% */ union { int16_t ta_offs_qbits; /* !< Burst TA Offset in quarter bits */ int16_t ta_offs_256bits;/*!< timing advance offset (in 1/256th bits) */ }; int16_t lqual_cb; /* !< Link quality in centiBel */ enum osmo_ph_pres_info_type pdch_presence_info; /*!< Info regarding presence/validity of header and data parts */ }; struct ph_tch_param { uint8_t chan_nr; /*!< Channel Number (Like RSL) */ uint32_t fn; /*!< GSM Frame Number */ int8_t rssi; /*!< RSSI of received indication */ uint8_t marker; /*!< RTP Marker bit (speech onset indicator) */ uint16_t ber10k; /*!< BER in units of 0.01% */ int16_t lqual_cb; /* !< Link quality in centiBel */ }; This is measurement related members are already presend in data or tch indication: * int8_t rssi; (same as inv_rssi?) * uint32_t fn; /*!< GSM Frame Number */ * uint16_t ber10k; /*!< BER in units of 0.01% */ This is what has to be added: * struct ph_tch_param lacks ta_offs_... members * both lack int16_t c_i_cb (seems to be unused!) * both lack uint8_t is_sub ========================== OPEN QUESTIONS ========================== (!) How do we handle the is_sub field? octphy does not set it. ==> who sets the flag? l1sap.c seems to read it only, but nobody seems to set it! ========================== IMPORTANT FUNCTIONS ========================== scheduler.c: int _sched_compose_ph_data_ind(struct l1sched_trx *l1t, uint8_t tn, uint32_t fn, enum trx_chan_type chan, uint8_t *l2, uint8_t l2_len, float rssi, int16_t ta_offs_256bits, int16_t link_qual_cb, uint16_t ber10k, enum osmo_ph_pres_info_type presence_info); int _sched_compose_tch_ind(struct l1sched_trx *l1t, uint8_t tn, uint32_t fn, enum trx_chan_type chan, uint8_t *tch, uint8_t tch_len); l1_if.c: int l1if_process_meas_res(struct gsm_bts_trx *trx, uint8_t tn, uint32_t fn, uint8_t chan_nr, int n_errors, int n_bits_total, float rssi, int16_t toa256);