Hi Patrik,

> Thanks for these patches. I'm going to add text from the cover letter to
> this patch and nitpick on clearing AutoConnect below.
> 
> On Thu, 2015-10-15 at 10:41 +0300, Jaakko Hannikainen wrote:
>> The previous implementation only cleared Error, which as per
>> documentation is wrong since Error is readonly. Add sane
>> defaults to all readwrite fields.
>> 
>> Reported by Francesco Bruno.
>> ---
>> src/service.c | 31 ++++++++++++++++++++++++++-----
>> 1 file changed, 26 insertions(+), 5 deletions(-)
>> 
>> diff --git a/src/service.c b/src/service.c
>> index e9107c2..fe20763 100644
>> --- a/src/service.c
>> +++ b/src/service.c
>> @@ -3582,11 +3582,32 @@ static DBusMessage *clear_property(DBusConnection 
>> *conn,
>>      dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &name,
>>                                                      DBUS_TYPE_INVALID);
>> 
>> -    if (g_str_equal(name, "Error")) {
>> -            set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
>> -
>> -            g_get_current_time(&service->modified);
>> -            service_save(service);
>> +    if (service->hidden) {
>> +            return __connman_error_not_supported(msg);
>> +    } else if (g_str_equal(name, "AutoConnect")) {
>> +            __connman_service_set_autoconnect(service, true);
> 
> Here I'd say clearing means to unset the value 'true'. I.e. the result
> is not that one gets a default value but the cleared one. Nitpicking on
> this means that the default value of AutoConnect is really false, it's
> only the DefaultAutoConnectTechnologies ones that are set to true.
> 
> I'll change this to avoid an extra round-trip.

why are we doing this at all. The reason why ClearProperty exists is so that 
you can clear read-only values like "Error" That is the only thing it is 
intended for. Nothing else.

For anything that that read-write this is pointless to support. Also if I 
wanted it to reset to default values, then it would have been called 
ResetProperty and not ClearProperty. It is just for clearing an error condition 
that ConnMan entered into where is has no capability to get itself out of. So 
the one time when user interaction is needed to clear an error.

Regards

Marcel

_______________________________________________
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Reply via email to