OsmoBTS reports ever-growing TA value
While writing some OsmoBTS tests in TTCN-3, I encountered the issue that the reported MS Timing Offset in RSL MEAS RES messages keeps increasing despite the MS never moving
I'm using trxcon+fake_trx, but this shouldn't matter. In this setup I can control the erported timing offset on uplink bursts. As soon as only the slightest timing offset is reported, the reported MS TO value keeps increasing magically by itself ?!?
It seems the culprit is Osmo-BTS Change-Id: I2e0dfd13b53e8aa2822985f12bf2985e683ab553 "fix measurement computation" which can be found at http://git.osmocom.org/osmo-bts/commit/?id=d5fdcfe6d95f52fb76c4f4201969347a062fc9fd
It introduces a new function
lchan_meas_update_ordered_TA whose functionality I still haven't yet managed to understand. It appears to be adjusting the requested timing advance (lchan->rqd_ta) but outside osmo-bts-trx/loops.c code. This is odd, as rqd_ta is a state variable of that loops.c code.
So for one part, it is a failure of encapsulation. The TA loop code should be self-contained, particularly as it is only used for omso-bts-trx, and not for the other BTS models. The new lchan_meas_update_ordered_TA() function is used from common code, applicable to all BTS models.The next part that strikes me about this function is that it
- has an input parameter "taqb_sum" which is actually not a sum but an average
- contains a computation like
ms_timing_offset = taqb_sum + (lchan->meas.l1_info - lchan->rqd_ta);where we mix
timing advance quarter-bitswith elements such as lchan->rqd_ta which is an unsigned integer in units of symbols not quarter-bits or quarter-symbols. I haven't looked at the unit of
lchan->meas.l1_infoyet, but I think it also is in full symbols, not quarters.
#2 Updated by laforge almost 2 years ago
- Status changed from New to In Progress
I'm going to revert the related commit now, as not only it doesn't seem to make sense to me regarding loops.c encapsulation, but it even has obvious problems in terms of mixing variables of different units in a computation.
#4 Updated by fixeria almost 2 years ago
I've experienced the same problem of continuously increasing
Timing Advance value, despite the ToA value remains the same.
It was happening only with the real hardware and regular
phones, while I couldn't reproduce the problem in fake_trx :/
Great that you managed to do that.
#7 Updated by dexter almost 2 years ago
The patch in question dates back to code that was contributed by Octasic inc. I have reviewed it and it seemed plausible. Unfortunately at that time there was no way to verify the correct functionality of timing advance so I had to rely on the review of Octasic.
No matter how I look at this patch, I can't figure out what it's trying to do ... AFAICT it's just plain wrong.
- It's a different timing loop that will obviously mess with the one in loop.c
- The algorithm has no hysteresis, it will inevitably oscillate
- AFAICT it mixes quarter-bit values (taqb) and full bits (TA values)
- The TOA for the last SACCH burst is going to be used twice in the average. Once added in rx_data_fn and then again in ta_val (via the call to trx_loop_sacch_input)
- When the reported used TA doesn't match the requested one, the loop does nothing (that's fine), but it should also probably clear the accumulated toa measurement in the channel state because those are for bursts sent with a different TA value and so are not really valid. Either we drop them and wait for the TA used by the phone to be updated, or we need to account for the difference between requested TA and actually used TA before we use them to determine if we should increase/decrease the TA
- The TOA values for TCH bursts (either speech or FACCH) don't seem to be used at all ?
But the patch above just doesn't fix any of those issues ...
I think this issue should have been fixed now. AFAIR, this bug could be (probably, still can be) triggered if at least one Measurement Report from the MS is lost. Last time I experienced this bug was when trxcon didn't support caching of Measurement Reports (dummy LAPDm frames were being sent instead). Now it does (see If1b8dc74ced746d6270676fdde75fcda32f91a3d, #2988).
Yeah, AFAIK the only thing left in this ticket was figure out the intent of the patch the Harald reverted to fix the growing TA issue.
And ... I don't think there is any. At least anymore. If this code did anything that the code in loop.c did at the time, that's not the case any more.
So unless someone has any objections, I would just close this.