Re: [PATCH 2/2] Code consitency for signal strength in HFP maemo6

Hi Dmitriy,

On Mon, Jul 12, 2010, Dmitriy Paliy wrote:

This patch simplifies and makes maemo6 telephony driver code consistent
with libscnet API regarding SignalBarsChanged. RSSI percents and RSSI
dBs are not among the parameters when SignalBarsChanged emited.
Therefore, these parameters are removed. Names are changed to reflect
libscnet d-bus interface notations. Comments and debug information are
updated in places where units or operations are unclear.
---
 audio/telephony-maemo6.c |   46 +++++++++++++++++++++++-----------------------
 1 files changed, 23 insertions(+), 23 deletions(-)

This patch has also been pushed upstream, but I had to make the
following manual fixes before that:

+	 /* Init as 0 meaning inactive mode. In modem power off state */
+	 /* can be be -1, but we treat all values as 0s regardless */ 
+	 /* inactive or power off. */

All three lines have tabs and spaces for intentation (one tab + one
space) and the second line has an extra space at the end before the
newline character.

+static void update_signal_strength(int32_t signal_strength_bars)

The variable name is unnecessarlily long. Simply signal_bars is better
imho.

+	if (signal_strength_bars < 0) {
+		DBG("signal strength smaller than expected: %d<0", signal_strength_bars);
+		signal_strength_bars = 0;
+	} else if (signal_strength_bars > 5) {
+		DBG("signal strength greater than expected: %d>5", signal_strength_bars);
+		signal_strength_bars = 5;

You're going beyond the 80 character limit here and the extra long
variable name isn't really helping out with that.

+	telephony_update_indicator(maemo_indicators, "signal", signal_strength_bars);

Same here.

Normally I'd have asked you to fix these things, but since we're trying
to get a new release out I went ahead and did it myself. So in the
future please pay attention to this kind of issues :)

Johan