neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31629 )
Change subject: cosmetic: assignment_fsm.c: naming tweaks
......................................................................
cosmetic: assignment_fsm.c: naming tweaks
Reading the code, I was confused: check_requires_voice_stream() sounds
like it returns a bool. Instead, it figures out chan modes and returns
success or error. Clarify.
Change-Id: Ib8230254dd8e0ea3cd9f65a5a2944989c56f8679
---
M src/osmo-bsc/assignment_fsm.c
1 file changed, 18 insertions(+), 5 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/29/31629/1
diff --git a/src/osmo-bsc/assignment_fsm.c b/src/osmo-bsc/assignment_fsm.c
index b9a7930..7d1dc58 100644
--- a/src/osmo-bsc/assignment_fsm.c
+++ b/src/osmo-bsc/assignment_fsm.c
@@ -369,7 +369,7 @@
OSMO_ASSERT(osmo_fsm_register(&assignment_fsm) == 0);
}
-static int check_requires_voice(bool *requires_voice, enum gsm48_chan_mode chan_mode)
+static int check_one_chan_mode_for_voice(bool *requires_voice, enum gsm48_chan_mode chan_mode)
{
*requires_voice = false;
@@ -392,7 +392,7 @@
/* Check if the incoming assignment request requires a voice stream or not,
* we will look at the preferred and the alternate channel mode and also make
* sure that both are consistent. */
-static int check_requires_voice_stream(struct gsm_subscriber_connection *conn)
+static int check_chan_modes_for_voice(struct gsm_subscriber_connection *conn)
{
bool requires_voice_pref = false, requires_voice_alt;
struct assignment_request *req = &conn->assignment.req;
@@ -405,7 +405,7 @@
* a mismatch is not permitted */
for (i = 0; i < req->n_ch_mode_rate; i++) {
- rc = check_requires_voice(&requires_voice_alt, req->ch_mode_rate_list[i].chan_mode);
+ rc = check_one_chan_mode_for_voice(&requires_voice_alt, req->ch_mode_rate_list[i].chan_mode);
if (rc < 0) {
assignment_fail(GSM0808_CAUSE_REQ_CODEC_TYPE_OR_CONFIG_NOT_SUPP,
"Channel mode not supported (prev level %d): %s", i,
@@ -529,8 +529,8 @@
assignment_count(CTR_ASSIGNMENT_ATTEMPTED);
/* Check if we need a voice stream. If yes, set the appropriate struct
- * members in conn */
- if (check_requires_voice_stream(conn) < 0)
+ * members in conn. On error, just return (assignment_fail() has already been invoked). */
+ if (check_chan_modes_for_voice(conn) < 0)
return;
if (!req->target_lchan && reuse_existing_lchan(conn)) {
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31629
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ib8230254dd8e0ea3cd9f65a5a2944989c56f8679
Gerrit-Change-Number: 31629
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: newchange
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31632 )
Change subject: tweak gsm_pchan_ids[]: s/OSMO_DYN/OSMODYN
......................................................................
tweak gsm_pchan_ids[]: s/OSMO_DYN/OSMODYN
This patch affects *only* on osmo_fsm instance IDs, which are visible on
the CTRL and VTY interfaces to identify FSM instances, and in the log.
Why bother: An upcoming patch wants to replace gsm_pchan_ids[] with
osmo_fsm_inst_update_id_f_sanitize(gsm_pchan_name(x)), this is an
explicit step to match gsm_pchan_names[].
Why drop the underscore: On the VTY, in phys_chan_config names, there
commonly is an underscore separator to indicate that a timeslot can
become either of the parts, e.g. TCH/F_PDCH means the timeslot can be
TCH/F, or it can be PDCH. The name "OSMO_DYN" would then imply that it
can be either OSMO, or DYN, so let's rather name it OSMODYN.
Change-Id: I4a540744cced466f0ca4fc605db4d0ec14ee8e87
---
M TODO-RELEASE
M src/osmo-bsc/gsm_data.c
M tests/bsc/bsc_test.ok
3 files changed, 27 insertions(+), 4 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/32/31632/1
diff --git a/TODO-RELEASE b/TODO-RELEASE
index d0852fc..ab85fc3 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -7,3 +7,4 @@
# If any interfaces have been added since the last public release: c:r:a + 1.
# If any interfaces have been removed or changed since the last public release: c:r:0.
#library what description / commit summary line
+ osmo-bsc CTRL,VTY osmo_fsm instance IDs are tweaked: OSMO_DYN becomes OSMODYN
diff --git a/src/osmo-bsc/gsm_data.c b/src/osmo-bsc/gsm_data.c
index 96c291d..33a878b 100644
--- a/src/osmo-bsc/gsm_data.c
+++ b/src/osmo-bsc/gsm_data.c
@@ -207,7 +207,7 @@
{ GSM_PCHAN_UNKNOWN, "UNKNOWN" },
{ GSM_PCHAN_CCCH_SDCCH4_CBCH, "CCCH_SDCCH4_CBCH" },
{ GSM_PCHAN_SDCCH8_SACCH8C_CBCH, "SDCCH8_CBCH" },
- { GSM_PCHAN_OSMO_DYN, "OSMO_DYN" },
+ { GSM_PCHAN_OSMO_DYN, "OSMODYN" },
{ 0, NULL }
};
diff --git a/tests/bsc/bsc_test.ok b/tests/bsc/bsc_test.ok
index bb255ad..f0adbd1 100644
--- a/tests/bsc/bsc_test.ok
+++ b/tests/bsc/bsc_test.ok
@@ -48,8 +48,8 @@
lchan->fi->id = 0-1-0-SDCCH8_CBCH-0
assignment.fi->id = msc4294967295-conn123_subscr-TMSI-0x00000423_0-1-0-SDCCH8_CBCH-0
pchan=OSMODYN:
- ts->fi->id = 0-1-0-OSMO_DYN
- lchan->fi->id = 0-1-0-OSMO_DYN-0
- assignment.fi->id = msc4294967295-conn123_subscr-TMSI-0x00000423_0-1-0-OSMO_DYNasNONE-0
+ ts->fi->id = 0-1-0-OSMODYN
+ lchan->fi->id = 0-1-0-OSMODYN-0
+ assignment.fi->id = msc4294967295-conn123_subscr-TMSI-0x00000423_0-1-0-OSMODYNasNONE-0
Testing execution completed.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31632
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I4a540744cced466f0ca4fc605db4d0ec14ee8e87
Gerrit-Change-Number: 31632
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: newchange
Attention is currently required from: fixeria, pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31615 )
Change subject: vty: add 'osmodyn' as alias for 'tch/f_tch/h_sdcch8_pdch'
......................................................................
Patch Set 1: Code-Review-1
(1 comment)
File src/osmo-bsc/gsm_data.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31615/comment/9a0f49f5_6cec40b9
PS1, Line 192: { GSM_PCHAN_OSMO_DYN, "OSMODYN" },
> not in the 'ts' / 'phys_chan_config' command we don't! see bts_trx_vty_init(). […]
(and of course i forgot to add a matching gsm_pchant_descs[] entry because the stupid gsm_pchan_ids[] is in between, hence the python tests fail, which a vty test would have made my realize.)
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31615
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I37719edd867c777d1ce944b8e2f1efffac38f00e
Gerrit-Change-Number: 31615
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 01 Mar 2023 22:48:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31615 )
Change subject: vty: add 'osmodyn' as alias for 'tch/f_tch/h_sdcch8_pdch'
......................................................................
Patch Set 1:
(1 comment)
File src/osmo-bsc/gsm_data.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31615/comment/dac921f8_93d18781
PS1, Line 192: { GSM_PCHAN_OSMO_DYN, "OSMODYN" },
> We use "OSMO_DYN", see line 209.
not in the 'ts' / 'phys_chan_config' command we don't! see bts_trx_vty_init().
oh, are you saying I should rather name it "OSMO_DYN"? I think that would be wrong, because in the phys_chan_config command, the underscore serves as a separator. Like "TCH/F_PDCH" is either "TCH/F" or "PDCH". "OSMO_DYN" would look like it is a pchan offering one of "OSMO" or "DYN".
(the "+" separator is about pchans that offer multiple things at the same time, the "_" is for alternative modes)
But I'm a bit puzzled, where do we use gsm_pchan_ids[] then?
... we only use it in timeslot_fsm.c, lchan_fsm.c, assignment_fsm.c. It is the CTRL-interface id compatible name for gsm_pchant_names. I guess we want to remove gsm_pchan_ids[], rather use gsm_pchant_names[] and use osmo_fsm_inst_update_id_f_sanitize() there? I'll see in a separate patch.
This confusion makes me notice that this patch should have a vty test to go with it.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31615
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I37719edd867c777d1ce944b8e2f1efffac38f00e
Gerrit-Change-Number: 31615
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 01 Mar 2023 22:45:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment