Attention is currently required from: arehbein, fixeria, laforge, neels.

pespin has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email )

Change subject: Introduce per-BTS timers, RLC timer commands
......................................................................


Patch Set 17:

(7 comments)

File include/osmocom/bsc/bts.h:

https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/410d863c_6d55ea01
PS12, Line 323: GSM_BTS_TDEF_ID_COUNTDOWN_VALUE
> as alluded to above, I was told it'd be good to use positive values for 
> anything coming from the spe […]
Not "anything coming from the specs", but counters/timers explicitly specified 
with a given T123 or N123 naming.
Hence, since Countdown value is not aexplicitly defined as a T... or N... it 
must be done as negative (X...).


File src/osmo-bsc/bsc_init.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/7fb0226f_ec45c056
PS16, Line 240:         bts_gprs_timer_groups_init(bts);
> (as mentioned above:) I have moved it to `gsm_bts_alloc()`; the function 
> description seemed like a b […]
Done


File src/osmo-bsc/bts.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/cb270503_135b1952
PS17, Line 466:         bts_gprs_timer_groups_init(bts);
did you make sure all counters set up here are not used beforehand in this 
function when initializing stuff? I bet some counter may be, so this should be 
far above.


File src/osmo-bsc/bts_init.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/987e30c1_d72af677
PS12, Line 60: 3101
> (this reply is older, hadn't yet published it...) […]
Done


File src/osmo-bsc/bts_init.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/7e893e33_fe82bedb
PS17, Line 57:    .desc = "\"In the extended uplink TBF mode, the uplink TBF 
may be maintained during temporary inactive periods, "
"Keep uplink TBF alive during inactive periods where the mobile station has no 
RLC information to transmit (3GPP TS 44.060 Version 6.14.0)"


https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/d3a42a61_ee58d91c
PS17, Line 61:    .desc = "A delayed release of the downlink TBF is when the 
release of the downlink TBF is delayed following the transmission of a final 
data block, "
"Delay release of downlink TBF after all available data has been transmitted"


File src/osmo-bsc/bts_vty.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/3e9bb2c4_44fcf5d0
PS3, Line 1706:         /* TODO (BSSGP timer patch): If argument is one of 
BSSGP Timers strings, then translate to
> what lines are you referring to? This is just a TODO comment for the BSSGP  
> timer patch
aha, I'll rephrase: so if it's a a patch about RLC, why are you adding a TODO 
about BSSGP timers here?

You can simplify this patch since you are anyway adding them in a follow up 
patch, it's not something left over "for later".



--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I2c24110d8c977d6cc74c3c8e77bcc709ad9d2675
Gerrit-Change-Number: 31878
Gerrit-PatchSet: 17
Gerrit-Owner: arehbein <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: neels <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-Attention: arehbein <[email protected]>
Gerrit-Attention: neels <[email protected]>
Gerrit-Attention: laforge <[email protected]>
Gerrit-Attention: fixeria <[email protected]>
Gerrit-Comment-Date: Tue, 05 Dec 2023 11:45:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: arehbein <[email protected]>
Comment-In-Reply-To: fixeria <[email protected]>
Comment-In-Reply-To: pespin <[email protected]>
Gerrit-MessageType: comment

Reply via email to