laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/21598 )
Change subject: gb: Add beginnings of a new BSSGP implementation ...................................................................... Patch Set 3: (3 comments) One of the questions where I don't have a clear feeling about is whether we should prefix also the BBV FSM with bssgp2_ instead of bssgp_. It's not like we had a FSM before, so there is no need to separate a new from an old implementatin in prefix. But then, mayb it makes sense to prefix all symbols introduced around the same time/generation of code with the same prefix? https://gerrit.osmocom.org/c/libosmocore/+/21598/3/src/gb/bssgp_bvc_fsm.c File src/gb/bssgp_bvc_fsm.c: https://gerrit.osmocom.org/c/libosmocore/+/21598/3/src/gb/bssgp_bvc_fsm.c@4 PS3, Line 4: /* FIXME: This should probably be moved to libosmogb? */ > it's already inlibosmogb right? Ack https://gerrit.osmocom.org/c/libosmocore/+/21598/3/src/gb/bssgp_bvc_fsm.c@70 PS3, Line 70: uint32_t advertised; > not important, but these can really be uint16_t then. AS you can see, today's spec already utilizes all 16 of those 16 bits. Wait for Rel16 or later defining more bits, and you will be happy we have used an uint32_t here from the start :P https://gerrit.osmocom.org/c/libosmocore/+/21598/3/src/gb/bssgp_bvc_fsm.c@283 PS3, Line 283: osmo_fsm_inst_state_chg(fi, BSSGP_BVCFSM_S_UNBLOCKED, T1_SECS, T1); > Not required, but you probably want to move to using osmo_tdef structures and > FSM APIs at some point […] There's actually a related FIXME in Line 25, thanks. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/21598 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Icbe8e4f03b68fd73b8eae95f6f6cccd4fa9af95a Gerrit-Change-Number: 21598 Gerrit-PatchSet: 3 Gerrit-Owner: laforge <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <[email protected]> Gerrit-Comment-Date: Tue, 08 Dec 2020 12:24:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <[email protected]> Gerrit-MessageType: comment
