osmo-bts[master]: LC15: Fix missing fill frame and GSM 05.08 mandatory frame

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Vadim Yanitskiy gerrit-no-reply at lists.osmocom.org
Tue Feb 13 15:42:36 UTC 2018


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 {
Here you can avoid the usage of else block, and thus reduce
the code nesting, because the function will return if DTX is
optional for particular combination of lchan and fn...


Line 502: 				LOGP(DL1C, LOGL_DEBUG, "%s Have to send %s %s zero speech frame, Fn=%d, Fn mod 104=%d, dump=%s\n",
This code block is already shifted away from the line
beginning, so I would use a single tab for such alignment.


Line 518: 				LOGP(DL1C, LOGL_DEBUG, "%s Have to send %s %s zero speech frame, Fn=%d, Fn mod 104=%d, dump=%s\n",
Same here, single tab for arguments would be better.


Line 534: 
Why do we have this line here?

It's not related to the change anyhow, so please remove.


Line 560: 		LOGP(DL1C, LOGL_DEBUG, "%s Have to send %s SID buffer "
Please also unify the alignment here...


-- 
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: 2
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen <minh-quang.nguyen at nutaq.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Minh-Quang Nguyen <minh-quang.nguyen at nutaq.com>
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-HasComments: Yes



More information about the gerrit-log mailing list