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/.
neels gerrit-no-reply at lists.osmocom.orgneels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/24777 ) Change subject: power_control: implement BCCH carrier power reduction operation ...................................................................... Patch Set 3: (15 comments) https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3//COMMIT_MSG Commit Message: https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3//COMMIT_MSG@14 PS3, Line 14: However, in the recent 3GPP TS 45.008 there is a feature called (could be nice to mention the spec version nr) https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3//COMMIT_MSG@30 PS3, Line 30: A value greater than zero would make osmo-bts reduce the power what do you mean, "would"? https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3/doc/manuals/chapters/power_control.adoc File doc/manuals/chapters/power_control.adoc: https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3/doc/manuals/chapters/power_control.adoc@334 PS3, Line 334: a BTS shall maintain discontinuous Downlink transmission at full power in order to (are you sure that you don't mean "continuous") https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3/doc/manuals/chapters/power_control.adoc@337 PS3, Line 337: version 13.0.0 (2015-11) - "BCCH carrier power reduction operation". "in version" https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3/doc/manuals/chapters/power_control.adoc@348 PS3, Line 348: and the simulation results can be found in 3GPP TR 45.926. ("TS"?) https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3/doc/manuals/chapters/power_control.adoc@352 PS3, Line 352: At the moment of writing this manual, the only BTS model that can instructed to (the usual expression is "at the time of writing") "that can be instructed" https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3/doc/manuals/chapters/power_control.adoc@354 PS3, Line 354: can be added on request sounds like it doesn't belong in this manual? if you need to say this, maybe rather say "may be added in the future", otherwise it sounds like osmocom is offering a free service of implementing things on request https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3/doc/manuals/chapters/power_control.adoc@358 PS3, Line 358: `show bts` command in the VTY interface. maybe "in OsmoBSC's VTY interface" https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3/doc/manuals/chapters/power_control.adoc@363 PS3, Line 363: There is currently no logic in osmo-bsc for automatic activation and deactivation (technically "osmo-bsc" is the binary and here it should be OsmoBSC ...) https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3/doc/manuals/chapters/power_control.adoc@407 PS3, Line 407: 3652121201381481804 maybe rather make it "1" ? otherwise it looks like a meaningful important number https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3/src/osmo-bsc/bsc_ctrl_commands.c File src/osmo-bsc/bsc_ctrl_commands.c: https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3/src/osmo-bsc/bsc_ctrl_commands.c@555 PS3, Line 555: const int red = atoi(value); atoi() is not sufficient to verify proper input. The value could be *anything*, supplied by an external user. atoi() would return some int value on error, e.g. if the user supplies value = "potatoe" or "1.234". A proper input value checking is found at e.g. https://git.osmocom.org/libosmocore/tree/src/gsm/gsm23003.c?id=d9825c0a2cfeab682f29167b11aaa154b8a36bae#n498 errno = 0; _mnc = strtol(mnc_str, &endptr, 10); if (errno) return -errno; else if (*endptr) return -EINVAL; if (_mnc < 0 || _mnc > 999) return -ERANGE; (slightly tweaked the example here to return -errno) https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3/src/osmo-bsc/bsc_ctrl_commands.c@584 PS3, Line 584: const int red = atoi(cmd->value); (here atoi() is fine because the verify function should have verified the value already) https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3/src/osmo-bsc/bts_osmobts.c File src/osmo-bsc/bts_osmobts.c: https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3/src/osmo-bsc/bts_osmobts.c@61 PS3, Line 61: dh = (struct abis_rsl_dchan_hdr *) msgb_put(msg, sizeof(*dh)); (usually no spaces in type cast) https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3/src/osmo-bsc/bts_osmobts.c@65 PS3, Line 65: dh->chan_nr = RSL_CHAN_BCCH; in the commit log you're saying this is abusing the message? maybe some comment here would be nice to indicate that it is an osmocom specific coding? https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3/tests/osmo-bsc.vty File tests/osmo-bsc.vty: https://gerrit.osmocom.org/c/osmo-bsc/+/24777/3/tests/osmo-bsc.vty@49 PS3, Line 49: c0-power-reduction BCCH carrier power reduction operation i would recommend also adding tests for the parameters in the way done below, e.g. 'bts 0 c0-power-reduction ?' to verify that the argument doc strings are matched up correctly -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/24777 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I047fce33d4d3e4c569dd006ba17858467a2f4783 Gerrit-Change-Number: 24777 Gerrit-PatchSet: 3 Gerrit-Owner: fixeria <vyanitskiy at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge at osmocom.org> Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de> Gerrit-CC: pespin <pespin at sysmocom.de> Gerrit-Comment-Date: Fri, 02 Jul 2021 15:52:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210702/b045b008/attachment.htm>