Attention is currently required from: jolly, laforge, pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33979 )
Change subject: ASCI: Add tests for voice group/broadcast calls at MSC
......................................................................
Patch Set 2:
(1 comment)
File msc/BSC_ConnectionHandler.ttcn:
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33979/comment/2ff27307_77e5…
PS2, Line 68: inout charstring, CallParameters, ASCI_Event;
As can be seen from the existing code, we tend to define strings as named constants and then send/match these constants rather than typing strings here and there. This is clearly better because this eliminates problems due to possible typos in strings.
```
COORD.send("CALL_READY");
...
COORD.receive("C[E]LL_READY");
```
So personally I really like the idea of using (valueless) enums and I vote for them.
> The problem with string typos can easiler be solved by definiting const string variables, or enums and converting them enum2str() when sending.
The problem is that... There is no `enum2str()` in TTCN-3.
What I don't like in the current patch revision is that we end up having testsuite specific stuff in a common module: `ASCI_Event` in this case. IMO, it's cleaner if each testsuite (`MSC_Tests`, `MSC_Tests_Iu`, and the new `BSC_Tests_ASCI`) would define its own COORD port and the related enums/types internally. This may not apply to the `MSC_Tests_Iu`, because it's mostly re-using the test logic from `MSC_Tests`, but I don't see the need for the `MSC_Tests[_Iu]` to have access to ASCI specific port messages.
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33979
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I4bbe739ea55ecf9f7ebf9ee413df69f29aa642f8
Gerrit-Change-Number: 33979
Gerrit-PatchSet: 2
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 31 Jul 2023 09:57:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/34005 )
Change subject: rlcmac: Initial selection of packet-access-procedure mode based on originating cause
......................................................................
rlcmac: Initial selection of packet-access-procedure mode based on originating cause
Change-Id: I930a1fcf23506f75562a6795f9a6e42b187d2974
---
M include/osmocom/gprs/rlcmac/llc_queue.h
M src/rlcmac/gre.c
M src/rlcmac/llc_queue.c
3 files changed, 43 insertions(+), 1 deletion(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
diff --git a/include/osmocom/gprs/rlcmac/llc_queue.h b/include/osmocom/gprs/rlcmac/llc_queue.h
index 7149f76..dd7528c 100644
--- a/include/osmocom/gprs/rlcmac/llc_queue.h
+++ b/include/osmocom/gprs/rlcmac/llc_queue.h
@@ -27,6 +27,7 @@
struct gprs_llc_prio_queue {
enum gprs_rlcmac_radio_priority radio_prio; /* Radio prio of this queue, range (1..4) */
+ enum osmo_gprs_rlcmac_llc_sapi sapi; /* LLC SAPI of this queue */
struct gprs_codel codel_state;
struct llist_head queue; /* queued LLC DL data. See enum gprs_rlcmac_llc_queue_prio. */
};
@@ -50,6 +51,7 @@
enum osmo_gprs_rlcmac_llc_sapi sapi, enum gprs_rlcmac_radio_priority radio_prio);
struct msgb *gprs_rlcmac_llc_queue_dequeue(struct gprs_rlcmac_llc_queue *q, bool can_discard);
enum gprs_rlcmac_radio_priority gprs_rlcmac_llc_queue_highest_radio_prio_pending(struct gprs_rlcmac_llc_queue *q);
+enum osmo_gprs_rlcmac_llc_sapi gprs_rlcmac_llc_queue_highest_llc_sapi_pending(struct gprs_rlcmac_llc_queue *q);
void gprs_rlcmac_llc_queue_merge_prepend(struct gprs_rlcmac_llc_queue *q, struct gprs_rlcmac_llc_queue *old_q);
diff --git a/src/rlcmac/gre.c b/src/rlcmac/gre.c
index 59cf0f5..0eb6a8c 100644
--- a/src/rlcmac/gre.c
+++ b/src/rlcmac/gre.c
@@ -198,6 +198,9 @@
/* Create a new UL TBF and start Packet access procedure to get an UL assignment if needed */
int gprs_rlcmac_entity_start_ul_tbf_pkt_acc_proc_if_needed(struct gprs_rlcmac_entity *gre)
{
+ enum osmo_gprs_rlcmac_llc_sapi tx_sapi;
+ enum gprs_rlcmac_tbf_ul_ass_type ul_ass_type;
+
/* TS 44.060 5.3 "In packet idle mode, upper layers may require the
* transfer of a upper layer PDU, which implicitly triggers the
* establishment of a TBF and the transition to packet transfer mode." */
@@ -212,8 +215,27 @@
gre->ul_tbf = gprs_rlcmac_ul_tbf_alloc(gre);
if (!gre->ul_tbf)
return -ENOMEM;
+
+ tx_sapi = gprs_rlcmac_llc_queue_highest_radio_prio_pending(gre->llc_queue);
+ /* 3GPP TS 44.018 3.5.2.1.2 "Initiation of the packet access procedure: channel request" */
+ switch (tx_sapi) {
+ case OSMO_GPRS_RLCMAC_LLC_SAPI_GMM:
+ /* "If the purpose [...] is to send a Page Response, a Cell update (the mobile
+ * station was in GMM READY state before the cell reselection) or for any other
+ * GPRS Mobility Management or GPRS Session Management procedure, the mobile station
+ * shall request a one phase packet access" */
+ ul_ass_type = GPRS_RLCMAC_TBF_UL_ASS_TYPE_1PHASE;
+ break;
+ default:
+ /* "If the purpose [...] is to send user data and the requested RLC mode is
+ * acknowledged mode, the mobile station shall request either a one phase packet
+ * access or a single block packet access." */
+ /* TODO: We always use 1phase for now... ideally we should decide
+ * based on amount of Tx data and configured MultiSlot Class? */
+ ul_ass_type = GPRS_RLCMAC_TBF_UL_ASS_TYPE_1PHASE;
+ }
/* We always use 1phase for now... */
- return gprs_rlcmac_tbf_ul_ass_start(gre->ul_tbf, GPRS_RLCMAC_TBF_UL_ASS_TYPE_1PHASE);
+ return gprs_rlcmac_tbf_ul_ass_start(gre->ul_tbf, ul_ass_type);
}
int gprs_rlcmac_entity_llc_enqueue(struct gprs_rlcmac_entity *gre,
diff --git a/src/rlcmac/llc_queue.c b/src/rlcmac/llc_queue.c
index 83c926e..d1f923a 100644
--- a/src/rlcmac/llc_queue.c
+++ b/src/rlcmac/llc_queue.c
@@ -47,6 +47,7 @@
for (i = 0; i < ARRAY_SIZE(q->pq); i++) {
for (j = 0; j < ARRAY_SIZE(q->pq[i]); j++) {
q->pq[i][j].radio_prio = i; /* enum gprs_rlcmac_radio_priority, range (0..3) */
+ q->pq[i][j].sapi = i + 1; /* osmo_gprs_rlcmac_llc_sapi, range (1..11) */
INIT_LLIST_HEAD(&q->pq[i][j].queue);
gprs_codel_init(&q->pq[i][j].codel_state);
}
@@ -222,6 +223,14 @@
return prioq->radio_prio;
}
+enum osmo_gprs_rlcmac_llc_sapi gprs_rlcmac_llc_queue_highest_llc_sapi_pending(struct gprs_rlcmac_llc_queue *q)
+{
+ struct gprs_llc_prio_queue *prioq = gprs_rlcmac_llc_queue_find_msg(q);
+ OSMO_ASSERT(prioq);
+ return prioq->sapi;
+}
+
+
/* Merge old_q messages into q, prepending them. old_q must be freed by the caller. */
void gprs_rlcmac_llc_queue_merge_prepend(struct gprs_rlcmac_llc_queue *q, struct gprs_rlcmac_llc_queue *old_q)
{
--
To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/34005
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: I930a1fcf23506f75562a6795f9a6e42b187d2974
Gerrit-Change-Number: 34005
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: arehbein, laforge.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33652 )
Change subject: ipa: Add segmentation callback
......................................................................
Patch Set 5:
(1 comment)
File include/osmocom/netif/ipa.h:
https://gerrit.osmocom.org/c/libosmo-netif/+/33652/comment/fda09d5a_08318f9e
PS4, Line 27: #define msgb_ipa_proto(__x) OSMO_MSGB_IPA_CB(__x)->proto
> I put `osmo_ipa_msgb_cb*`, because it's in `ipa.h` and not in `msgb. […]
I am really not following. You say one thing for the define in line 26 while saying the contrari for defines in lines 27 and 28? really?
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33652
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I87ef4c7023126b783dd79e7ed47be31e1b76f975
Gerrit-Change-Number: 33652
Gerrit-PatchSet: 5
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Mon, 31 Jul 2023 09:34:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: jolly, laforge, fixeria.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33979 )
Change subject: ASCI: Add tests for voice group/broadcast calls at MSC
......................................................................
Patch Set 2:
(1 comment)
File msc/BSC_ConnectionHandler.ttcn:
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33979/comment/b7612a08_d14d…
PS2, Line 68: inout charstring, CallParameters, ASCI_Event;
> I would go the other way around: Why are there strings at all, and not more specific types going thr […]
the messages being passed over COORD are in general quite test-specific, and the port shouldn't be seen as a regular protocol-defined port, but simply some sort of synchronization method imho, hence why simply using strings is enough and makes stuff simpler imho.
The problem with string typos can easiler be solved by definiting const string variables, or enums and converting them enum2str() when sending.
That being said, if others prefer ending up with several unrelated record types being transmitted over that same port I won't be blocking it despite disagreeing in principle.
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33979
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I4bbe739ea55ecf9f7ebf9ee413df69f29aa642f8
Gerrit-Change-Number: 33979
Gerrit-PatchSet: 2
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 31 Jul 2023 09:31:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/33935 )
(
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: osmo-bts-trx: bts_supports_cm_data(): allow non-transparent modes
......................................................................
osmo-bts-trx: bts_supports_cm_data(): allow non-transparent modes
The rate adaptation algorithm is the same for both transparent and
non-transparent channel modes. The only difference is that the
E-bits in the modified CSD frames carry data, like the D-bits.
Change-Id: Ib0d9346d7a8e30b8a8e4b08ee04846ba7d12b3fb
Related: OS#1572
---
M src/common/bts.c
1 file changed, 17 insertions(+), 0 deletions(-)
Approvals:
osmith: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
laforge: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/src/common/bts.c b/src/common/bts.c
index 1f0040c..40fe354 100644
--- a/src/common/bts.c
+++ b/src/common/bts.c
@@ -875,11 +875,14 @@
switch (bts->variant) {
case BTS_OSMO_TRX:
switch (cm->chan_rate) {
+ /* TODO: RSL_CMOD_CSD_NT_14k5 */
/* TODO: RSL_CMOD_CSD_T_14k4 */
+ case RSL_CMOD_CSD_NT_12k0:
case RSL_CMOD_CSD_T_9k6:
if (cm->chan_rt != RSL_CMOD_CRT_TCH_Bm)
return false; /* invalid */
/* fall-through */
+ case RSL_CMOD_CSD_NT_6k0:
case RSL_CMOD_CSD_T_4k8:
case RSL_CMOD_CSD_T_2k4:
case RSL_CMOD_CSD_T_1k2:
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/33935
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ib0d9346d7a8e30b8a8e4b08ee04846ba7d12b3fb
Gerrit-Change-Number: 33935
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged