Making jolly/testing ready to be merged?

[Holger Hans Peter Freyther]

On Sat, Jan 04, 2014 at 10:01:15AM +0100, Andreas Eversberg wrote:

Andreas,

> we need this conversion, since we also need "cell_options" for SI3 and
> SI6. i just use this attribute (and convert it back), so i don't need to
> add an extra attribute to "bts" structure.

Okay,

I would prefer macros/inline functions that convert from the numerical
value to the on-wire representation. It is a complain I have with the
PCU code and here as well. Here it might look like overkill.

In terms of Software Engineering it is all about information hiding to
reduce the complexity. One way to do this is to think about what secrets
a method/block/component/module has/should have. E.g. in the nanoBTS patch
method it knows about how to patch the tables we have but it should not
necessarily know about the different forms of representation of the
radio link timeout. Or in the more general note. Not all parts of the
code should know everything.


-	bts->si_common.cell_options.radio_link_timeout = 7; /* 12 */
+	bts->si_common.cell_options.radio_link_timeout = 7; /* Use RADIO LINK TIMEOUT of 32 */


+	vty_out(vty, "  radio-link-timeout %d%s",
+		(bts->si_common.cell_options.radio_link_timeout + 1) << 2,
+		VTY_NEWLINE);

+	bts->si_common.cell_options.radio_link_timeout = (atoi(argv[0])>>2) - 1;


+	/* patch CON_FAIL_CRIT */
+	nanobts_attr_bts[13] =
+		(bts->si_common.cell_options.radio_link_timeout + 1) << 2;
+

E.g. you could do something like this in OpenBSC

static inline int get_radio_link_timeout(struct gsm_bts *bts)
{
	return (bts->si_common.cell_options.radio_link_timeout + 1) << 2
}

static inline void set_radio_link_timeout(struct gsm_bts *bts, int value)
{
	bts->si_common.cell_options.radio_link_timeout = (value>>2) - 1;
}

and the above code would become..


+	set_radio_link_timeout(bts, 32);
+	vty_out(vty, "  radio-link-timeout %d%s",
+		get_radio_link_timeout(bts), VTY_NEWLINE);
+	nanobts_attr_bts[13] = get_radio_link_timeout(bts);

it is shorter, more expressive, comes at no code increase (in striped binaries),
hides the secret of the conversion in a single place, etc.


holger