Attention is currently required from: neels.
dexter has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-pcu/+/31176
)
Change subject: support for Ericsson RBS E1 CCU
......................................................................
Patch Set 15:
(10 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/60a6536f_51baf6aa
PS13, Line 10: BTS
BSC ?
Done
https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/30fc01fb_3adf33fd
PS13, Line 15: fun
fun?? =)
Done
File src/ericsson-rbs/er_ccu_if.c:
https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/fcf7ca0a_f683c037
PS13, Line 62: /* called by trau frame synchronizer: feed received MAC blocks into PCU */
does it make sense to document who calls the function?
usually we just describe what it does?
There are multiple callbacks in this c file
used by different entities (->demux->synchronizer->) I think the comment helps to
make sense since it helps to follow the path better.
https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/f7899371_902a94cd
PS13, Line 76: multeiplexer
multiplexer
Done
https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/e7cb34e8_267457ec
PS13, Line 90: multeiplexer
i
Done
https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/efa69b66_b4c70a28
PS13, Line 99: Function to
(drop the first two words)
Done
https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/7eacc5f3_850e9866
PS13, Line 122: ,
(nicer to start a new line for osmo_hexdump)
I
think its better to have it in one line. I agree its a bit long but too many lines also
make the log more difficult to read. Especially when grepping for things.
https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/eed0b53e_ffe2f48a
PS13, Line 143: (line=%u,ts=%u)
seems this log context occurs in several places,
welcome a LOG_FOO() macro instead of code dup. […]
There is already a LOGPCCU.
(line=%u,ts=%u) is only used three times where I couln'd use LOGPCCU.
https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/0a5a2e99_e0674cc1
PS13, Line 146: Note
"Note"? =)
Done
https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/01dd58cb_73f7fad0
PS13, Line 184: /* NOTE: This is a serious error: subslots should be removed when
l1if_close_pdch() is called by the PCU. This
(seems this comment wants to be inside the if { .. }
body.) […]
Done
--
To view, visit
https://gerrit.osmocom.org/c/osmo-pcu/+/31176
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I5c0a76667339ca984a12cbd2052f5d9e5b0f9c4d
Gerrit-Change-Number: 31176
Gerrit-PatchSet: 15
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Feb 2023 10:00:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment