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/.
pespin gerrit-no-reply at lists.osmocom.orgpespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/18903 )
Change subject: Introduce bts_shutdown FSM
......................................................................
Patch Set 1:
(5 comments)
https://gerrit.osmocom.org/c/osmo-bts/+/18903/1/include/osmo-bts/bts_shutdown_fsm.h
File include/osmo-bts/bts_shutdown_fsm.h:
https://gerrit.osmocom.org/c/osmo-bts/+/18903/1/include/osmo-bts/bts_shutdown_fsm.h@29
PS1, Line 29: ST_BTS_SHUTDOWN_NONE
> Cosmetic: BTS_SHUTDOWN_ST_* and BTS_SHUTDOWN_EV_* is more usual the ST_*. Not critical.
Ack
https://gerrit.osmocom.org/c/osmo-bts/+/18903/1/include/osmo-bts/bts_shutdown_fsm.h@41
PS1, Line 41: bts_shutdown
> Rather move this API to bts.{c,h}, so there will be no need to include this header everywhere.
I explicitly moved it here since I prefer having everything shutdown related in one file. I know, we need to export more headers in some files, but let's start splitting stuff into different headers rather than going the gsm_data.h way with kilometric headers.
https://gerrit.osmocom.org/c/osmo-bts/+/18903/1/src/common/bts_shutdown_fsm.c
File src/common/bts_shutdown_fsm.c:
https://gerrit.osmocom.org/c/osmo-bts/+/18903/1/src/common/bts_shutdown_fsm.c@51
PS1, Line 51: st_bts_shutdown_wait_ramp_down_compl_on_enter
> Given that it's a static (private) symbol, its name can be much shorter. […]
I think it's usual practice using the whole state name as function name for FSM. I'll perhaps drop the "bts_shutdown" part and leave "st_"
https://gerrit.osmocom.org/c/osmo-bts/+/18903/1/src/common/bts_shutdown_fsm.c@68
PS1, Line 68: llist_for_each_entry_reverse
> Would be nice to add a comment explaining why we doing reverse here.
I'm simply moving this loop from the old bts_shutdown() implementation here, so no need to change anything in this patch.
https://gerrit.osmocom.org/c/osmo-bts/+/18903/1/src/common/bts_shutdown_fsm.c@130
PS1, Line 130: constructor
> Constructors are usually placed at the bottom of the file.
I'm rather keeping all FSM internal stuff together and below this all public APIs using it.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/18903
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I8f48f17e61c3b9b86342eaf5b8a2b1ac9758bde5
Gerrit-Change-Number: 18903
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Comment-Date: Thu, 18 Jun 2020 14:16:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy at sysmocom.de>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200618/0ef49412/attachment.htm>