Pau Espin Pedrol has posted comments on this change. (
https://gerrit.osmocom.org/5753 )
Change subject: LC15: Fix missing fill frame and GSM 05.08 mandatory frame
..
Patch Set 9: Code-Review-1
(1 comment)
https://gerrit.osmo
Hello Vadim Yanitskiy, Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/5753
to look at the new patch set (#8).
LC15: Fix missing fill frame and GSM 05.08 mandatory frame
It was discovered that the LC15 BTS does not send L2 fill frame
in case th
Patch Set 7:
(2 comments)
https://gerrit.osmocom.org/#/c/5753/7/src/common/msg_utils.c
File src/common/msg_utils.c:
Line 489: if (!!lchan->tch.dtx.len)
imho we can simplify the !!.
https://gerrit.osmocom.org/#/c/5753/7/src/osmo-bts-litecell15/l1_if.c
File src/osmo-bts-litecell15
Hello Vadim Yanitskiy, Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/5753
to look at the new patch set (#7).
LC15: Fix missing fill frame and GSM 05.08 mandatory frame
It was discovered that the LC15 BTS does not send L2 fill frame
in case th
Patch Set 6:
(1 comment)
https://gerrit.osmocom.org/#/c/5753/6/src/common/msg_utils.c
File src/common/msg_utils.c:
Line 489: if (!!lchan->tch.dtx.len)
> I am still unsure about this use of '!!'.
This is basically done to transform an integer into a boolean, ie: var>=0 ->
var=1, b
Patch Set 6:
(1 comment)
https://gerrit.osmocom.org/#/c/5753/6/src/common/msg_utils.c
File src/common/msg_utils.c:
Line 489: if (!!lchan->tch.dtx.len)
> I am still unsure about this use of '!!'.
Oops, I meant to say: "why not write it as 'dtx.len != 0'"
--
To view, visit https:
Patch Set 6:
(1 comment)
https://gerrit.osmocom.org/#/c/5753/6/src/common/msg_utils.c
File src/common/msg_utils.c:
Line 489: if (!!lchan->tch.dtx.len)
I am still unsure about this use of '!!'.
It looks much like an accidental typo where '!' was intended.
Or is it really a check
Patch Set 6:
I have applied the purely cosmetic suggestions from my comment above.
--
To view, visit https://gerrit.osmocom.org/5753
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet
Hello Vadim Yanitskiy, Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/5753
to look at the new patch set (#6).
LC15: Fix missing fill frame and GSM 05.08 mandatory frame
It was discovered that the LC15 BTS does not send L2 fill frame
in case th
Patch Set 5:
(8 comments)
I'm not familiar with these parts of the spec. So my comments are only cosmetic
in nature.
https://gerrit.osmocom.org/#/c/5753/5/src/common/msg_utils.c
File src/common/msg_utils.c:
Line 388: else
Add curly braces to this else block? It spans more than o
Patch Set 5: Verified-1
--
To view, visit https://gerrit.osmocom.org/5753
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 5
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Ow
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/5753
to look at the new patch set (#5).
LC15: Fix missing fill frame and GSM 05.08 mandatory frame
It was discovered that the LC15 BTS does not send L2 fill frame
in case there is nothing to
Patch Set 4: Verified-1
I've refactored the source code to fit the common Osmocom
code style, but have no opportunity to verify and test.
URGENT: please check everything before submitting!
--
To view, visit https://gerrit.osmocom.org/5753
To unsubscribe, visit https://gerrit.osmocom.org/settin
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/5753
to look at the new patch set (#4).
LC15: Fix missing fill frame and GSM 05.08 mandatory frame
It was discovered that the LC15 BTS does not send L2 fill frame
in case there is nothing to
Patch Set 1:
> (1 comment)
Thanks for your comments. I will refactor the codes to make them cleaner at
some point.
--
To view, visit https://gerrit.osmocom.org/5753
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29e
Patch Set 3:
(1 comment)
https://gerrit.osmocom.org/#/c/5753/3//COMMIT_MSG
Commit Message:
Line 13: Related issue: https://osmocom.org/issues/1950
Please move close to the change-id and change to:
Related: OS#1950
--
To view, visit https://gerrit.osmocom.org/5753
To unsubscribe, visit http
Patch Set 3: Code-Review-1
And finally, regarding to the commit message: we usually follow
a shorter line length in description, so I would be happy if you
limit it as other contributers do. But this is cosmetics...
-1 due to some code warnings...
--
To view, visit https://gerrit.osmocom.org/5
Patch Set 2:
(6 comments)
https://gerrit.osmocom.org/#/c/5753/2/src/common/msg_utils.c
File src/common/msg_utils.c:
Line 484: if (lchan->tch_mode != GSM48_CMODE_SPEECH_AMR) {
Moreover, I would recommend to use a switch here:
switch (lchan->tch_mode) ...
Line 487: else {
H
Hello Vadim Yanitskiy, Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/5753
to look at the new patch set (#2).
LC15: Fix missing fill frame and GSM 05.08 mandatory frame
Problem:
We have noticed that the LC15 BTS does not send L2 fill frame in
Patch Set 1:
> > ping. Minh-Quang do you plan to submit a new version soon? seems
> > like a few minutes job but it's been waiting for over a month.
>
> Unfortunately, my local branch of this topic has been lost... is
> there anyway to pull this topic to my local BTS repository?
Just ignore
Patch Set 1:
> ping. Minh-Quang do you plan to submit a new version soon? seems
> like a few minutes job but it's been waiting for over a month.
Unfortunately, my local branch of this topic has been lost... is there anyway
to pull this topic to my local BTS repository?
--
To view, visit http
Patch Set 1:
> ping. Minh-Quang do you plan to submit a new version soon? seems
> like a few minutes job but it's been waiting for over a month.
Hi Paul,
I am sorry for the delay. I think that we can simply replace the static const
by const declaration in msg_util.h.
--
To view, visit http
Patch Set 1:
Do we need the same fix for sysmoBTS as well?
--
To view, visit https://gerrit.osmocom.org/5753
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I40e9bf9438c0b400e4d29eb39ffae37207e34db6
Gerrit-PatchSet: 1
Gerrit-Project: osmo
Patch Set 1:
ping. Minh-Quang do you plan to submit a new version soon? seems like a few
minutes job but it's been waiting for over a month.
--
To view, visit https://gerrit.osmocom.org/5753
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id
Patch Set 1: Code-Review-1
(1 comment)
https://gerrit.osmocom.org/#/c/5753/1/include/osmo-bts/msg_utils.h
File include/osmo-bts/msg_utils.h:
PS1, Line 35: static const uint8_t gsm_speech_zero[GSM_FR_BYTES] = {
: 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00,
Review at https://gerrit.osmocom.org/5753
LC15: Fix missing fill frame and GSM 05.08 mandatory frame
Problem:
We have noticed that the LC15 BTS does not send L2 fill frame in case there is
nothing to transmit.
This leads to bad RXQUAL repored by MS during signaling in TCH channel.
Related
26 matches
Mail list logo