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
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
