Dev (#6) - Python Support (#25) - Message List

Python Support
 unsolved

Starting a new topic for python support...

  • Message #103

    I posted the code I have at PythonWrapper. I ported it over to the new API, and I think it's working alright. Sometimes exceptions don't propagate up to the interpreter correctly, which can cause segfaults, but I'm sure it's a small issue.

    There are large chunks of functionality still unimplemented. This should provide a good framework, however.

    I took some liberties on the python side to make it a little more pythonic. Since there aren't really switch statements in python, I instead have the user associate "actions" (python functions) with any type of message they wish to receive (eg. MESG_BTN). That makes the python side a little more adaptable as well since you don't need to add onto a switch statement in python every time the message types change in C. I also removed the CWIID prefixes since they are redundant in an object oriented environment.

    • Message #104

      Awesome. And I thought I was the man for toggling LEDs with SWIG last night...

      I'll play around with this as soon as I get a chance - but it may be Sunday. I'm on the fence about the actions idea - not because it isn't a good one, but because I want to keep the Python and C interfaces as close as possible (modulo explicit object-oriented syntax in Python).

      In any case, kudos and much gratitude.

      • Message #105

        That makes sense. It's really an extension anyway since the python wrapper is just a convenient extension on top of the capabilities already provided in the cwiidmodule. I could easily move that up a layer in my code to achieve the same cleanliness without actually affecting the API.In fact, the python layer really shouldn't do anything except contain all those #defines.

        • Message #107

          OK, I'm still reading up on extension and embedding, so bear with me...

          You've got a good start - a few notes:

          1. cwiidmodule.c seems to be the convention, but call the module cwiid, and the class Wiimote. This makes the object of discourse cwiid.Wiimote, mirroring the C type cwiid_wiimote_t. cwiidmodule.cwiidmodule is confusing.
          2. According to the documentation, Python.h _must_ be included first.
          3. Be a bit more consistent in naming - I want to be able to look at a function name and tell the class for which it's implementing a member function - I'd suggest Wiimote_{read,write,etc}.

          What's your goal here? Are you handing this off to me, or do you want to stick around and help maintain it?

          Thanks again for your work.

          • Message #111

            I'll help maintain it for the time being. Eventually I'll hand it off, but I'm still interested for now! I'll probably be doing some work on this over the weekend, so I'll incorporate the changes you mentioned.

            • Message #112

              Excellent - I've got a few changes, but until we get this in svn, I can hold off, or send them to you after your next iteration.

              While you're in there, take a closer look at the multi-threading. As I understand it, you only need to call the GILState functions when you're playing with python objects in a thread other than the one in which the interpreter runs, which means our only GIL-critical region is the callback bridge.

          • Message #113

            I refactored the code and removed the unnecessary locks. I also removed the actions from the python side.

            I did leave the C module as cwiidmodule though. Since in the libcwiid library, all the functions and constants are prefixed with cwiid_, I thought it would be appropriate if the same prefix worked in python. Therefore I kept cwiid as the name of the python module and cwiidmodule the name of the C bindings.Unfortunately, the interpreter seems to get confused if I name the cwiidpy.py file cwiid.py and try to import cwiid. I think it's trying to import the cwiidmodule. We could do away with the python file entirely if we defined all the constants in the C module itself, but I'm feeling lazy so I'm not going to do that today.

            I removed the class on the Python side since it was unnecessary.

            • Message #114

              Also, if we removed the python proxy file, it would make sense to rename cwiidmodule cwiid

            • Message #115

              I also implemented the cwiid_read function. It's now been uploaded

              • Message #116

                I just realized there's a memory leak in there since I never free the buffer returned from the libcwiid read. I don't have time to fix it now, but you should be aware that it's there.

                • Message #117

                  I threw a free(buf) in there. The code is checked into branches/cwiidpy, I'm about to send you the password for the branch.

                  I made a few changes, mostly stylistic. The big change was adding the constants to cwiidmodule.c, deleting cwiidpy.py, and changing the module name to cwiid. Left the filename cwiidmodule.c - the Python documentation lists that as standard, and the interpreter apparently searches for x.so and xmodule.so.

                  • Message #118

                    Implemented a couple more functions...

                  • Message #119

                    Thanks for the SVN access.

                    As for the free(buf), you can't actually free that buffer until the PyObject? it returns is done with. The problem is, I don't see any way of knowing when that is. I have a feeling you're really not supposed to return those buffers up to unknown python code. Perhaps instead of a buffer, we should return a tuple that is composed of every byte requested. Seems pretty bad, but that's essentially what I'm doing in my app now. When I get that buffer I just unpack it with struct.unpack() and use the resulting tuple to calibrate the acceleration parameters. For now I just checked in code that doesn't free, since it won't break things using it.

                    Your changes look good, though I noticed you put all my function headers on 1 line instead of 2. Now how am I supposed to grep for function headers?? :)

                    • Message #120

                      Spoke too soon, I can't commit :(.

                      • Message #121

                        I put all the function headers on 1 line since that is how the rest of CWiid is formatted. I'm not averse to changing styles if there's a good reason - what's the advantage of the 2 line style?

                        • Message #122

                          The advantage of the 2-line style is that you can search for the name of a function with grep (or any regexp search) and only find where it's actually implemented, rather than everywhere it's called:

                          grep "function_name" *

                          will find where the function name is the first word on the line, which should only be the function itself if you use the 2 line style.

                          Don't change from what you're comfortable with, I'm happy to stand on the sidelines and criticize. I can work with anything.

                          • Message #123

                            Let's leave it how it is for now. Also, forums are wiki-formatted, put triple curly braces around verbatim text to avoid The Strangeness (caret is exponentiation).

                            I'll figure out what's wrong with your svn access when I get home from work. Might even test it this time.

                            • Message #128

                              Committed some more changes - finished up the state and processMesg, get_mesg calls. A couple of points about the message formats:

                              • All of the enumerations are currently numerics - I like the idea of returning strings as you had it, but I'd like to set up a type for them that has both string and numeric representations.
                              • I'm not sure about the structure for some of the message types - for messages that only have one member (acc, ir), do we really need a top level dictionary? But then keeping a top level dictionary for every message type is more consistent. Another possibility is representing the entire mesg array as a dictionary of messages. There's currently no condition under which multiple message of the same type are delivered at the same time. However, this doesn't match the C interface very well...
                              • Should we eventually define types for some of these things (state, messages)? Fortunately, you can set up the mapping protocol for types, so it'll be backward compatible with the current dictionaries.

                              Thoughts? If you're actively working on this, let me know which parts so we can avoid stepping on each other's toes.

                              • Message #141

                                Sorry I haven't been around. I got busy for a while there.

                                - I agree with being able to do both strings and numerics. How hard is that to do? Would pure numerics and pseudo-enumerations be good enough?

                                - I do not see a need to have messages be represented as a dictionary of messages since random access isn't needed often. Normally every new batch of messages requires some sort of processing for each message, so you want to iterate through each message anyway. There's no reason to keep them as a dictionary in that case.

                                - A mapping protocol sounds like a good way to mantain backwards compatibility, but I don't think that's very important right now. I think we should get something that works well with the smallest amount of code possible. Then as people start to use it we can add things to keep newer versions backwards compatible.

                                As you may have guessed, I'm not actively working on this right now. I may get back to it in a week or two. I'm not as busy anymore though, so I'll keep my eyes on the forum.

                                • Message #146

                                  No problem.

                                  1. Not too hard - just extend the int class.
                                  2. Agreed. It's still a list.
                                  3. Agreed. That's what beta/pre-1.0 versions are for, right? Breaking stuff!

                                  Let me know before you start working on it again. I don't plan to use the cwiidpy branch anymore, as I'm integrating it into the distribution.

Attachments

No attachments created.