Hi Denis,
On 29 November 2016 at 17:49, Denis Kenzior <denkenz(a)gmail.com> wrote:
Hi Djalal,
On 11/25/2016 08:00 AM, djalal(a)endocode.com wrote:
>
> From: Djalal Harouni <djalal(a)endocode.com>
>
> Handle the previously added types in D-Bus.
> ---
> src/netmon.c | 40 +++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/src/netmon.c b/src/netmon.c
> index eb18b9c..c87980a 100644
> --- a/src/netmon.c
> +++ b/src/netmon.c
> @@ -78,7 +78,8 @@ void ofono_netmon_serving_cell_notify(struct
> ofono_netmon *netmon,
> const char *technology = cell_type_to_tech_name(type);
> char *mcc = NULL;
> char *mnc = NULL;
> - int intval;
> + char *op = NULL;
> + int intval = -1;
Why the initializer? Our rule of thumb is not to initialize in order to let
the compiler (hopefully) or valgrind catch instances of uninitialized
variable use (e.g. bugs).
Ok dropped that one too, but in this case it does not matter, va_arg()
is undefined any way...
> netmon->reply = dbus_message_new_method_return(netmon->pending);
>
> if (netmon->reply == NULL)
> @@ -180,6 +181,43 @@ void ofono_netmon_serving_cell_notify(struct
> ofono_netmon *netmon,
> intval, uint8_t, DBUS_TYPE_BYTE);
> break;
>
> + case OFONO_NETMON_INFO_RSCP:
> + intval = va_arg(arglist, int);
> +
> + CELL_INFO_DICT_APPEND(&dict,
> "ReceivedSignalCodePower",
> + intval, uint8_t, DBUS_TYPE_BYTE);
> + break;
> +
> + case OFONO_NETMON_INFO_ECN0:
> + intval = va_arg(arglist, int);
> +
> + CELL_INFO_DICT_APPEND(&dict,
> "ReceivedEnergyRatio",
> + intval, uint8_t, DBUS_TYPE_BYTE);
> + break;
> +
> + case OFONO_NETMON_INFO_RSRQ:
> + intval = va_arg(arglist, int);
> +
> + CELL_INFO_DICT_APPEND(&dict,
> "ReferenceSignalReceivedQuality",
> + intval, uint8_t, DBUS_TYPE_BYTE);
> + break;
> +
> + case OFONO_NETMON_INFO_RSRP:
> + intval = va_arg(arglist, int);
> +
> + CELL_INFO_DICT_APPEND(&dict,
> "ReferenceSignalReceivedPower",
> + intval, uint8_t, DBUS_TYPE_BYTE);
> + break;
> +
This looks fine, however there should be a patch documenting these and their
respective value ranges inside doc/netmon-api.txt.
OK documented in netmonmonitor-api.txt of v2 of the patches. Thank you!
> + case OFONO_NETMON_INFO_OPERATOR:
> + op = va_arg(arglist, char *);
> +
> + if (op && strlen(op))
> + ofono_dbus_dict_append(&dict,
"Operator",
> + DBUS_TYPE_STRING, &op);
> +
> + break;
> +
> case OFONO_NETMON_INFO_INVALID:
> break;
> }
>
Regards,
-Denis