Attention is currently required from: pespin, fixeria.
Hello Jenkins Builder, fixeria,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-pcu/+/31649
to look at the new patch set (#5).
Change subject: pcu_l1_if_phy: support multiple BTS (in theory)
......................................................................
pcu_l1_if_phy: support multiple BTS (in theory)
The PCU already has a list that can hold multiple BTS objects but the
API for the direct PHY access has no way to associate a PHY with a
certain BTS object. Lets update the API so that we can associate a BTS
object when opening a PDCH.
Unfortunately OsmoPCU has never been tested with more than one BTS so it
is very likely that there are still shortcomings that prevent OsmoPCU to
work properly when more then one BTS is attached. To make users aware of
this, also print a warning as soon as more than one BTS object exists.
Related: OS#5198
Related: OS#5930
Change-Id: I33518cbbe83f7f8c071afa5e995d30930099ba92
---
M src/osmo-bts-litecell15/lc15_l1_if.c
M src/osmo-bts-oc2g/oc2g_l1_if.c
M src/osmo-bts-sysmo/sysmo_l1_if.c
M src/pcu_l1_if.cpp
M src/pcu_l1_if_phy.h
5 files changed, 29 insertions(+), 5 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/49/31649/5
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31649
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I33518cbbe83f7f8c071afa5e995d30930099ba92
Gerrit-Change-Number: 31649
Gerrit-PatchSet: 5
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: fixeria, dexter.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31497 )
Change subject: pcu_sock: activate/deactivate PDCH on pcu reconnect
......................................................................
Patch Set 9:
(1 comment)
File src/osmo-bsc/timeslot_fsm.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/fe703c49_c84d4181
PS9, Line 347: }
> it is very easy and quick to put the code move in a separate patch preceding this patch, so that the […]
ok so i added a move patch, pushed on branch neels/for_dexter; you can rebase your branch like this:
git fetch
git checkout pmaier/e1gprs
git rebase -i origin/neels/for_dexter
and when you hit the merge conflict just run this to resolve with no effort:
git checkout --theirs src/osmo-bsc/timeslot_fsm.c
git add src/osmo-bsc/timeslot_fsm.c
git rebase --continue
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31497
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I9ea0c53a5e68a51c781ef43bae71f947cdb95678
Gerrit-Change-Number: 31497
Gerrit-PatchSet: 9
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 03 Mar 2023 15:27:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: fixeria, dexter.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31649 )
Change subject: pcu_l1_if_phy: support multiple BTS (in theory)
......................................................................
Patch Set 4:
(1 comment)
File src/pcu_l1_if.cpp:
https://gerrit.osmocom.org/c/osmo-pcu/+/31649/comment/5a84dc92_a2e30375
PS4, Line 735: LOGP(DL1IF, LOGL_ERROR, "more then one BTS regsitered at this PCU. This PCU has only been tested with one BTS! OS#5930\n");
More than
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31649
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I33518cbbe83f7f8c071afa5e995d30930099ba92
Gerrit-Change-Number: 31649
Gerrit-PatchSet: 4
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 03 Mar 2023 15:22:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: pespin, fixeria.
dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31649 )
Change subject: pcu_l1_if_phy: support multiple BTS (in theory)
......................................................................
Patch Set 4:
(1 comment)
File src/pcu_l1_if.cpp:
https://gerrit.osmocom.org/c/osmo-pcu/+/31649/comment/913a6c28_4227a289
PS3, Line 1143: if (llist_count(&the_pcu->bts_list) > 1)
> what? do you want to count the BTS list and print every time a DATA_IND is received? […]
Done
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31649
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I33518cbbe83f7f8c071afa5e995d30930099ba92
Gerrit-Change-Number: 31649
Gerrit-PatchSet: 4
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 03 Mar 2023 15:17:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin, fixeria.
Hello Jenkins Builder, fixeria,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-pcu/+/31649
to look at the new patch set (#4).
Change subject: pcu_l1_if_phy: support multiple BTS (in theory)
......................................................................
pcu_l1_if_phy: support multiple BTS (in theory)
The PCU already has a list that can hold multiple BTS objects but the
API for the direct PHY access has no way to associate a PHY with a
certain BTS object. Lets update the API so that we can associate a BTS
object when opening a PDCH.
Unfortunately OsmoPCU has never been tested with more than one BTS so it
is very likely that there are still shortcomings that prevent OsmoPCU to
work properly when more then one BTS is attached. To make users aware of
this, also print a warning as soon as more than one BTS object exists.
Related: OS#5198
Related: OS#5930
Change-Id: I33518cbbe83f7f8c071afa5e995d30930099ba92
---
M src/osmo-bts-litecell15/lc15_l1_if.c
M src/osmo-bts-oc2g/oc2g_l1_if.c
M src/osmo-bts-sysmo/sysmo_l1_if.c
M src/pcu_l1_if.cpp
M src/pcu_l1_if_phy.h
5 files changed, 29 insertions(+), 5 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/49/31649/4
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31649
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I33518cbbe83f7f8c071afa5e995d30930099ba92
Gerrit-Change-Number: 31649
Gerrit-PatchSet: 4
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: neels, fixeria, dexter.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/31145 )
Change subject: bts: add IMMEDIATE ASSIGNMENT via PCH transmission
......................................................................
Patch Set 20:
(2 comments)
File include/osmocom/pcu/pcuif_proto.h:
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/21e6ff55_31827fa5
PS17, Line 280: uint8_t pgroup[3];
> I think we should transfer the whole IMSI to osmo-bsc. […]
Send whole IMSI to BTS/BSC: fine let's to that.
File src/bts.cpp:
https://gerrit.osmocom.org/c/osmo-pcu/+/31145/comment/4fbcef59_77987b64
PS17, Line 1134: pcu_l1if_tx_pch(bts, immediate_assignment, plen, ms_paging_group(tbf_ms(tbf)));
> It seems that the discussion is a bit stuck here. I suggest the following compromise: […]
Agree with the first part: Add the new message sending the whole IMSi and keep the backward compatibility in this patch and we will also bump the version number
Disagree with the second part: The fact that we still support the older version of the protocol with the older version of the message in osmo-pcu doesn't mean we need to bump the version again to drop the old message. From protocol point of view, the old message doesn't exist anymore after the first bump. It's just an implementation thing of osmo-pcu to keep supporting the older version of the protocol which still uses the old message.
Moreover, let's keep this backward compatibility (allow use of older protocol version) for a while to let osmo-pcu work with older osmo-bts for a while.
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31145
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I2a78651593323e8b9627c39918d949a33497b70f
Gerrit-Change-Number: 31145
Gerrit-PatchSet: 20
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 03 Mar 2023 15:09:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: fixeria, dexter.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31497 )
Change subject: pcu_sock: activate/deactivate PDCH on pcu reconnect
......................................................................
Patch Set 9:
(8 comments)
File doc/timeslot.msc:
https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/e9a8d2f6_ea8faa96
PS2, Line 95: bsc_ts abox bsc_ts [label="UNUSED"];
> Thats true, Can we shorten this since the mechanism is already explained above. […]
(Dots are not sufficient to indicate an omission, they mean to wait for an async event.)
https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/c59c7118_1204e859
PS2, Line 98: "
> Done
(I actually meant literally "\n" to force a line break, but i tested and it looks fine like this)
https://people.osmocom.org/neels/kes0Chie/timeslot.png
File doc/timeslot.msc:
https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/7999f20f_f7833d3a
PS9, Line 95: bsc_ts box bsc_ts [label="release PDCH"];
: ...;
ok i'll be happy if you replace these two lines with
bts note bsc_ts [label="PDCH release, see WAIT_PDCH_DEACT above"]
(i.e. use a "note" and explicitly refer to another place, and span across all the lines involved)
https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/a6d0b606_0a495a6c
PS9, Line 104: bsc_ts box bsc_ts [label="activate PDCH"];
: ...;
...and replace these two lines with
bts note bsc_ts [label="PDCH activation, see WAIT_PDCH_ACT above"]
File src/osmo-bsc/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/f28c813f_a2d43bcd
PS9, Line 803: * not need any extra activation/deactivation of PDCHs. */
i thought more like this:
/* BSC co-located PCU applies only to Ericsson RBS, which supports only GSM_PCHAN_OSMO_DYN.
* So we need to deact only on this pchan kind. */
https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/83938bf7_8265ec42
PS9, Line 974: See comment above)
it is not clear what this refers to ... just drop this? or explicitly say what you mean to say.
I think you mean
/* BSC co-located PCU applies only to Ericsson RBS, which supports only GSM_PCHAN_OSMO_DYN.
* So we need to act only on this pchan kind. */
File src/osmo-bsc/timeslot_fsm.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/913feae1_ae0495ed
PS9, Line 347: }
it is very easy and quick to put the code move in a separate patch preceding this patch, so that the addition of 'is_ericsson_bts()' is not "hidden" in the moved code block. i would appreciate that, also for future readers of the git log. You can even +2 that patch yourself saying "trivial" -- because it would be only a cosmetic move of unchanged code. I could also do that for you if it helps...
https://gerrit.osmocom.org/c/osmo-bsc/+/31497/comment/857b9996_9020a13d
PS9, Line 1128: NOTE
i'm really puzzled why you seem to start every comment everywhere with "NOTE:" -- it is completely redundant and has no meaning at all
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31497
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I9ea0c53a5e68a51c781ef43bae71f947cdb95678
Gerrit-Change-Number: 31497
Gerrit-PatchSet: 9
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 03 Mar 2023 15:05:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: comment