Attention is currently required from: osmith.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bsc/+/31622
to look at the new patch set (#2).
Change subject: requires_voice_stream -> ch_indctr
......................................................................
requires_voice_stream -> ch_indctr
Use the full gsm0808_chan_indicator value throughout the lchan related
structs (assignment_fsm_data, gsm_lchan, lchan_activate_info,
lchan_modify_info) instead of reducing it to the boolean
requires_voice_stream.
This is needed so we don't lose the information whether an lchan was
requested for data or speech (both need an rtp stream).
Add a new bsc_chan_ind_requires_rtp_stream function and use it in
conditionals like the previous requires_voice_stream.
Related: OS#4393
Change-Id: I1538c1e6d5cd61559af7c1e2860afd0269dda367
---
M doc/lchan.msc
M include/osmocom/bsc/gsm_08_08.h
M include/osmocom/bsc/gsm_data.h
M include/osmocom/bsc/lchan.h
M src/osmo-bsc/assignment_fsm.c
M src/osmo-bsc/bsc_vty.c
M src/osmo-bsc/gsm_08_08.c
M src/osmo-bsc/handover_fsm.c
M src/osmo-bsc/lchan_fsm.c
M src/osmo-bsc/osmo_bsc_bssap.c
10 files changed, 92 insertions(+), 56 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/22/31622/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31622
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I1538c1e6d5cd61559af7c1e2860afd0269dda367
Gerrit-Change-Number: 31622
Gerrit-PatchSet: 2
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: osmith.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31622 )
Change subject: requires_voice_stream -> ch_indctr
......................................................................
Patch Set 1:
(2 comments)
Patchset:
PS1:
You could even move this function to libosmocore.
File include/osmocom/bsc/gsm_08_08.h:
https://gerrit.osmocom.org/c/osmo-bsc/+/31622/comment/8f1c90b3_15082d52
PS1, Line 16: bool bsc_requires_rtp_stream(enum gsm0808_chan_indicator ch_indctr);
this function name sounds weird. Name seems to indicate it's some sort of global config, but you are actually simply checking stuff on chan_ind.
chan_ind_requires_rtp_stream sounds more accurate?
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31622
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I1538c1e6d5cd61559af7c1e2860afd0269dda367
Gerrit-Change-Number: 31622
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 01 Mar 2023 14:33:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: osmith.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31620 )
Change subject: rsl: put values for Channel Mode into enums
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31620
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I25bfd02aa1428a35492b20376a31635a442e545f
Gerrit-Change-Number: 31620
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 01 Mar 2023 14:31:23 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: laforge.
osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31549 )
Change subject: rsl_tx_ipacc_crcx/mdcx: omit speech mode for CSD
......................................................................
Patch Set 3:
(1 comment)
File src/osmo-bsc/abis_rsl.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31549/comment/4d25103f_d2cf15ac
PS2, Line 2747: lchan->abis_ip.rtp_payload == RTP_PT_CSDATA
> not entirely sure it's a good idea to use the RTP payload type as criteria. […]
With the current lchan struct it would only be possible to check for CSD by checking if lchan->current_ch_mode_rate.chan_mode is any of GSM48_CMODE_DATA_*. This is also just a derivative value, the information originally comes from the channel indicator (data, speech, signalling) in the BSSAP assignment request.
I've added a patch to store the channel indicator in the struct, and used it here.
-> https://gerrit.osmocom.org/c/osmo-bsc/+/31622/1
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31549
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I6641b713177276bcf798f08123e1dd2e88ffdce6
Gerrit-Change-Number: 31549
Gerrit-PatchSet: 3
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Wed, 01 Mar 2023 14:30:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: comment
Attention is currently required from: laforge.
osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31545 )
Change subject: lchan.h: remove enum lchan_csd_mode
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://gerrit.osmocom.org/c/osmo-bsc/+/31545/comment/3834bb0a_2952484e
PS2, Line 10: But instead we can just
: directly translate the BSSAP value to the RSL value.
> we can, but do we already have a related enum in the ts48_058 header files?
The values were there, but not yet as enum. Changed it with this patch:
https://gerrit.osmocom.org/c/libosmocore/+/31620
> The 48.058 split into transparent and non-transparent also makes it slightly more complex to understand, as the uint8_t can have different meanings.
Changed it to an union of both enums.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31545
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ice914744da3a2084e82d125848fb69404b8e8a36
Gerrit-Change-Number: 31545
Gerrit-PatchSet: 3
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Wed, 01 Mar 2023 14:29:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: comment
Attention is currently required from: laforge.
osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31547 )
Change subject: bssmap_handle_ass_req_ct_data: implement
......................................................................
Patch Set 2:
(3 comments)
File src/osmo-bsc/codec_pref.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31547/comment/918a15ee_6af94be3
PS2, Line 117: gsm0804
> gsm0408, not gsm0804. 04.x is radio/Um interface, while 08.x is RAN interfaces like Abis or A.
Done
https://gerrit.osmocom.org/c/osmo-bsc/+/31547/comment/04eeac17_fa82a25b
PS2, Line 137: sm0804
> likewise here, gsm0408. or gsm48 if you want to stay like the enum/constant values.
Done
File src/osmo-bsc/osmo_bsc_bssap.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/31547/comment/28a388cb_ffc4bf9d
PS2, Line 677: select_codecs_data
> FYI, it's not really a 'codec' in the data case, but a "rate". […]
agreed, renamed the functions and moved them to data_rate_pref.c
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31547
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I350bea15fd2158eb6edc9bc92f2dca48930736e9
Gerrit-Change-Number: 31547
Gerrit-PatchSet: 2
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Wed, 01 Mar 2023 14:29:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: comment
Attention is currently required from: osmith.
Hello Jenkins Builder, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bsc/+/31545
to look at the new patch set (#3).
Change subject: lchan.h: remove enum lchan_csd_mode
......................................................................
lchan.h: remove enum lchan_csd_mode
It looks like the idea was to translate the CSD rate from BSSAP to
lchan_csd_mode before translating it to RSL. But instead we can just
directly translate the BSSAP value to the RSL value.
The previous code was not used yet (nothing wrote to csd_mode).
Related: OS#4393
Depends: libosmocore I25bfd02aa1428a35492b20376a31635a442e545f
Change-Id: Ice914744da3a2084e82d125848fb69404b8e8a36
---
M include/osmocom/bsc/lchan.h
M src/osmo-bsc/abis_rsl.c
2 files changed, 29 insertions(+), 61 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/45/31545/3
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31545
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ice914744da3a2084e82d125848fb69404b8e8a36
Gerrit-Change-Number: 31545
Gerrit-PatchSet: 3
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-MessageType: newpatchset