Python bindings

[Aleksander Morgado]

>>>>> 1/ client.get_manufacturer() won't take None as the first argument, but
>>>>> looking at the gir I can't see why not. As a hack I passed in a function
>>>>> reference, knowing that its unused anyway.
>>>>
>>>> Maybe some missing (allow-none) annotation for the 'input' variable in
>>>> the async call?
>>>>
>>>>> 2/ Where necessary, complex return values should be returned in a tuple.
>>>>> If
>>>>> you look at get_manufacturer_ready() you'll see that supposedly the
>>>>> value
>>>>> is returned in the string passed in to arg1. For me that's always empty,
>>>>> but I checked with qmicli, the value is populated properly. The page
>>>>> https://live.gnome.org/PyGObject/IntrospectionPorting suggests that it
>>>>> should be returning tuples in this case, and its certainly more
>>>>> pythonic.
>>>>
>>>> Maybe some missing (out) annotation for each variable returned when
>>>> reading the TLV from the output bundle?
>>>
>>> Hi Aleksander,
>>>
>>>     Given that GetManufacturer is in the generated stuff, where should I
>>>     apply
>>>
>>> my changes so that they appear in its output?
>>
>> Directly in the code generator sources; under build-aux/qmi-codegen/*.py
> 
> Aleksander,
>    I've found and made the changes and now get output now. One thing though 
> the unused parameter instead of 'allow-none', could be 'skip', which seems 
> nicer. On the other hand I'm not sure if you ever intend to use this 
> parameter?
> 

That's the main problem. Messages defined without input TLVs need the
NULL input bundle in the API, just in case some day the message is
updated with a first TLV added. There are definitely cases where this
will not happen; e.g. GetManufacturer will very likely never need an
input TLV. But we cannot tell about *all* messages without input TLVs.
Hence, I would personally leave it as (allow-none) instead of directly
(skip).



> Below are the patches for discussion:
> 
> ###############
> 
>>From 35b5bb2916b36c17350c3ded24d34b97d4592b2d Mon Sep 17 00:00:00 2001          
> From: Andrew Bird <ajb at spheresystems.co.uk>                                     
> Date: Fri, 16 Nov 2012 11:29:39 +0000                                           
> Subject: [PATCH 1/2] Add annotation to output strings in the generated output   
>                                                                                 
> This patch ensures that the bindings recognise that these                       
> variables are dealt with as output ones. On Python this                         
> changes the calling convention to be more pythonic and enables                  
> the value to be returned e.g.                                                   
>                                                                                 
> before:                                                                         
>         s = ''                                                                  
>         v = m.get_manufacturer(s)                                               
>         # s is still empty                                                      
>                                                                                 
> after:                                                                          
>         (v, s) = m.get_manufacturer()                                           
>         # s == "QUALCOMM INCORPORATED"                                          
> ---                                                                             
>  build-aux/qmi-codegen/VariableString.py | 2 +-                                 
>  1 file changed, 1 insertion(+), 1 deletion(-)                                  
>                                                                                 
> diff --git a/build-aux/qmi-codegen/VariableString.py b/build-aux/qmi-
> codegen/VariableString.py
> index a127eb7..96f49f4 100644                                                   
> --- a/build-aux/qmi-codegen/VariableString.py                                   
> +++ b/build-aux/qmi-codegen/VariableString.py                                   
> @@ -258,7 +258,7 @@ class VariableString(Variable):                             
>                           'name' : variable_name }                              
>                                                                                 
>          template = (                                                           
> -            '${lp}@${name}: a placeholder for the output constant string, or 
> %NULL if not required.\n')
> +            '${lp}@${name}: (out): a placeholder for the output constant 
> string, or %NULL if not required.\n')
>          return string.Template(template).substitute(translations)              
>                                                                      


So, this fixes it for strings. The other variable types would need the
same update. Are you able to add those as well?


>                                                                                 
> --                                                                              
> 1.7.11.7                                                                        
> 
> ###################
> 
>>From 0147e1fdcb4eeac7cb761ccf7069753f8848b45c Mon Sep 17 00:00:00 2001          
> From: Andrew Bird <ajb at spheresystems.co.uk>                                     
> Date: Fri, 16 Nov 2012 11:59:49 +0000                                           
> Subject: [PATCH 2/2] Allow unused parameters to have null passed in             
>                                                                                 
> ---                                                                             
>  build-aux/qmi-codegen/Client.py | 2 +-                                         
>  1 file changed, 1 insertion(+), 1 deletion(-)                                  
>                                                                                 
> diff --git a/build-aux/qmi-codegen/Client.py b/build-aux/qmi-codegen/Client.py  
> index 5fa4178..7a95d29 100644                                                   
> --- a/build-aux/qmi-codegen/Client.py                                           
> +++ b/build-aux/qmi-codegen/Client.py                                           
> @@ -286,7 +286,7 @@ class Client:                                               
>              if message.input.fields is None:                                   
>                  input_arg_template = 'gpointer unused'                         
>                  translations['input_var'] = 'NULL'                             
> -                translations['input_doc'] = 'unused: %NULL. This message 
> doesn\'t have any input bundle.'
> +                translations['input_doc'] = 'unused: (allow-none): %NULL. 
> This message doesn\'t have any input bundle.'
>              else:                                                              
>                  input_arg_template = '${input_camelcase} *input'               
>                  translations['input_var'] = 'input'                            

Nothing else to comment abuot this one.


Some of the things I'm not sure about is which is the default 'transfer'
mode for variables if none defined. Is it full? or none?

The input and output bundles have new(),ref() and unref(). In the case
of new() and ref() they should be transfer full; I don't think I added
those. Then, for all set/get operations in the bundles, the variables
passed are all transfer-none. So seems to me that either one or the
other also needs annotation tags to specify the correct transfer mode.
Are you able to look at that as well?


You're coming with a good set of patches here :-) If you have a public
repo somewhere with all these in a branch let me know.

-- 
Aleksander