[PATCH evemu 15/19] py: Add base class LibraryWrapper

[Peter Hutterer]

On Tue, Jan 07, 2014 at 11:22:11AM +0100, Daniel Martin wrote:
> On Tue, Jan 07, 2014 at 09:46:18AM +1000, Peter Hutterer wrote:
> > On Mon, Jan 06, 2014 at 06:38:15PM +0100, Daniel Martin wrote:
> > > LibraryWrapper will be a base class for others wrapping a shared
> > > library with ctypes.
> > > 
> > > And add various functions (i.e. expect_ge_zero()), which will be used as
> > > callback functions to check the return value of an API call.
> > > 
> > > Signed-off-by: Daniel Martin <consume.noise at gmail.com>
> > > ---
> > >  python/evemu/base.py | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 128 insertions(+)
> > > 
> > > diff --git a/python/evemu/base.py b/python/evemu/base.py
> > > index 713aec1..3a9de8d 100644
> > > --- a/python/evemu/base.py
> > > +++ b/python/evemu/base.py
> > > @@ -1,11 +1,139 @@
> > > +"""
> > > +The base module provides classes wrapping shared libraries.
> > > +"""
> > >  import ctypes
> > >  import ctypes.util
> > >  import os
> > >  
> > > +# Import types directly, so they don't have to be prefixed with "ctypes.".
> > > +from ctypes import c_char_p, c_int, c_uint, c_void_p
> > > +
> > >  import evemu.const
> > >  import evemu.exception
> > >  
> > >  
> > > +def expected_or_error(result, func, args, is_expected):
> > > +    """
> > > +    Raise an ExecutionError for an unexpected result (is_expected == True).
> > 
> > this commment seems off - the error is raised if is_expected == False.
> 
> Ups.
> 
> > how about calling this "raise_error_if" and reordering the parameters? that
> > way the caller code would be
> > 
> >     def expect_eq_zero(result, func, args):
> >         """ Expect 'result' being equal to zero. """
> >         return raise_error_if(result != 0, result, func, args)
> > 
> > I think that's more self-explanatory than the current approach, and you can
> > rename "is_expected" to "raise_error". and the code becomes
> > if raise_error:
> >    ...
> > else:
> >    return args
> 
> Will be changed.
> 
> > btw: returning "args" seems a bit odd, is that intended?
> 
> Yes:
>     http://docs.python.org/3.3/library/ctypes.html
>     ...
>     If the errcheck function returns the argument tuple it receives
>     unchanged, ctypes continues the normal processing it does on the
>     output parameters.
>     ...
> I have added this as a comment to the code.
> 
> If you don't return args as it came in, then the foreign function call
> returns the modified value instead of the original return value.

how odd. but ok, thanks for adding a comment. we're not really python
developers, so oddities like that are great to have documented.

> > not sure how the passing of objects works exactly but you may be able to
> > have expect_eq, expect_ne, expect_gt and the actual expected value as
> > attribute on the function somewhere.
> > e.g. in the declaration call something like
> > 
> >         "evemu_new": {
> >             "argtypes": (c_char_p,),
> >             "restype": c_void_p,
> >             "comparison": expect_ne,
> >             "retval": None
> >         }
> > 
> > and in the loading call something like
> >    api_call.errcheck = attrs["comparison"]
> >    api_call.retval = attrs["retval"]
> > and then use this here (provided you still have that object, I didn't try)
> 
> Yes, that's possible. This would allow us to remove expect_not_null().
> But, on the other hand it requires to add the expected value at every
> "prototype" next to the comparision function we've to specify anyways.
> 
> I'd stick with this solution until more advanced comparisions are
> necessary, i.e. range checks.

no worries, fair call.

Cheers,
   Peter