Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve parameters parsing and fix callbacks #125

Merged
merged 12 commits into from
Apr 29, 2020
Merged

Improve parameters parsing and fix callbacks #125

merged 12 commits into from
Apr 29, 2020

Conversation

albestro
Copy link
Contributor

Close #117

As per discussion in the issue, I tried to improve a bit the parser for parameters. It is far from being perfect.

The new method should is not regex based, it should be able to detect all types and pointers with related constness. The constness information is stored inside Par as a list of bools, intrinsically giving two information:

  • it is a value (len==0), a pointer (len==1) or a pointer to pointer (len==2)
  • constness for each existing level of indirection

e.g.

  • const int * -> [True, False]
  • const int * const -> [True, True]
  • int * const -> [False, True]

@albestro
Copy link
Contributor Author

albestro commented Mar 31, 2020

For the moment I worked to be able to reproduce (more or less) the same generated wrappers.

Here it is a diff between what I generated with master 943e80a and this branch

1579c1579
<     MediaReadCb = ctypes.CFUNCTYPE(ctypes.POINTER(ctypes.c_ssize_t), ctypes.c_void_p, ctypes.POINTER(ctypes.c_char), ctypes.c_size_t)
---
>     MediaReadCb = ctypes.CFUNCTYPE(ctypes.POINTER(ctypes.c_ssize_t), ctypes.c_void_p, ctypes.c_char_p, ctypes.c_size_t)
5876c5876
<                     ctypes.c_int, Media, MediaSlaveType, ctypes.c_uint, ctypes.c_char_p)
---
>                     ctypes.c_int, Media, MediaSlaveType, ctypes.c_int, ctypes.c_char_p)
5902c5902
<                     ctypes.c_uint, Media, ctypes.POINTER(ctypes.POINTER(MediaSlave)))
---
>                     ctypes.c_int, Media, ctypes.POINTER(ctypes.POINTER(MediaSlave)))
5913c5913
<                     None, ctypes.POINTER(MediaSlave), ctypes.c_uint)
---
>                     None, ctypes.POINTER(MediaSlave), ctypes.c_int)
7250c7250
<                     None, MediaPlayer, Position, ctypes.c_uint)
---
>                     None, MediaPlayer, Position, ctypes.c_int)
7659c7659
<                     ctypes.c_int, MediaPlayer, ctypes.c_uint, ctypes.c_char_p, ctypes.c_uint, ctypes.c_uint)
---
>                     ctypes.c_int, MediaPlayer, ctypes.c_uint, ctypes.c_char_p, ctypes.c_int, ctypes.c_int)

Some highlights:

  • some int are now recognized as unsigned int (added the new type mapping)
  • added in type2class a "wrong" mapping for unsigned char* to not alter too much the wrapper (at least for the moment)
  • added in type2class_out the "right" mapping unsigned char* to ctypes.POINTER(ctypes.c_char) to make the callback media_read_cb get the right wrapper function signature
  • in Par.flags member function, forced unsigned char* to be recognized as Flag.Out (for the same reason of the previous point)

Copy link
Owner

@oaubert oaubert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @albestro , thanks. It all looks good to me, apart from the small remark in the review. Could you fix this, so that I can simply merge the pull request as-is? The cherry on the cake would be to add a test in the tests/test.py file if it you can figure out one, but this is more optional.

generator/generate.py Outdated Show resolved Hide resolved
@albestro
Copy link
Contributor Author

albestro commented Apr 21, 2020

I've refactored the parse_param function by moving it to class Par and making it a @classmethod function.

Moreover, I implemented some very basic tests for parse_param and I improved it.

There is also a minor fix in another test.

@albestro albestro marked this pull request as ready for review April 21, 2020 21:09
@albestro albestro requested a review from oaubert April 21, 2020 21:09
@oaubert oaubert merged commit 354ba8e into oaubert:master Apr 29, 2020
@oaubert
Copy link
Owner

oaubert commented Apr 29, 2020

Awesome contribution, many thanks. I just merged it.

@albestro albestro deleted the fix_callbacks branch May 17, 2020 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

media_new_callbacks: c_char_p for foreign buffer
2 participants