osmo-bts[master]: Extend Get Attribute responder

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/.

Harald Welte gerrit-no-reply at lists.osmocom.org
Mon Jun 5 06:41:24 UTC 2017


Patch Set 3: Code-Review-1

(3 comments)

https://gerrit.osmocom.org/#/c/2786/3/src/common/oml.c
File src/common/oml.c:

Line 178: 	if (power <= 0)
a negative or zero dBm value might be a bit unusual but well within the capabilities of most hardware and used in the context of e.g. testing or direct wired/cabled connections.  get_p_max_out_mdBm() should be used in this context, with no fall-back. If it results in invalid values, it's a bug by osmo-bts.

OML will reduce that power by max_pwr_red, but applying it prematurely here doesn't make sense. The terminology in the specs is unfortunately not always consistent.


Line 187: 	msgb_tv_put(msg, NM_ATT_MANUF_STATE, power);
why are we encoding the power in an IE that is called STATE ?!?


Line 191: static inline int cleanup_attr_msg(struct msgb *msg, int length, uint8_t *out)
* we generally have the output argument as first, just like memcpy()
* we generlaly use 'const *' for input arguments
* 'length' normally implies the length of the output buffer. If you mean an offset into the buffer, call it accordingly

Also, an honest question: why do we pass attr_out_index+1 into the function, only to remove that byte here in the function body?


-- 
To view, visit https://gerrit.osmocom.org/2786
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f72305bbf1ab74745bffac1bee9f539f5a6de32
Gerrit-PatchSet: 3
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: Yes



More information about the gerrit-log mailing list