machinekit / machinetalk-protobuf

Protobuf declarations for machinekit messages
MIT License
10 stars 11 forks source link

Python packaging/namespacing #22

Closed bobvanderlinden closed 8 years ago

bobvanderlinden commented 9 years ago

As discussed in https://github.com/bobvanderlinden/pymachinetalk-client/issues/1, it would be nice to have namespacing/directory structure so that machinetalk-protobuf can be packaged on its own without conflicting with other packages.

The protobuf library itself is an example of how to do the namespacing/packaging and how to handle each language: https://github.com/google/protobuf

bobvanderlinden commented 9 years ago

I've tried to replicate this for Python, so that a package can be generated using setup.py. This should make it possible to put it on pypi, so that people can just pip install machinetalk-protobuf. It is not yet ready, since the Makefile is now broken, but I'll handle that next.

https://github.com/bobvanderlinden/machinetalk-protobuf/commit/d16c8aa94a780594d6b2c5bbb2f3b485a9d6cb72

mhaberler commented 9 years ago

wow ;)

bobvanderlinden commented 9 years ago

Haha, thanks! It should be very similar to https://github.com/google/protobuf/tree/master/python and https://github.com/google/protobuf/tree/master/python/setup.py. But, it is possible to install setup.py to a virtualenv (or globally on the system) and use it thereafter with from machinetalk_protobuf.message_pb2 import Container (yay).

bobvanderlinden commented 9 years ago

I did some more work on getting things running properly on my system: https://github.com/bobvanderlinden/machinetalk-protobuf

This version fixes the makefile for C++ and the dependency/object files. The C++ headers can be included using "machinetalk/protobuf/message.pb.h", instead of just "message.pb.h". This is the same convention that is used for Protobuf itself: "google/protobuf/message.h". We might want to go for "machinetalk/protobuf/message.h" (dropping ".pb" from the filename), but it doesn't have any significant benefit.

Also, I think it might be a good idea to compile the .cc files into a static/shared object that can be linked to. It seems protobuf itself also does this, but I'm not sure whether this has negative consequences.

I still need to add an install rule to install all files in the correct places on the system.

I wasn't entirely sure where the dependency files (.d) were used for. Since I couldn't generate those earlier I thought they were meant to be d source code files (the D programming language). What are they exactly used for?

The changes should also work for Javascript, but I have not tried this yet, because I lack proto2js. It seems proto2js is also deprecated for protobuf.js 4: https://github.com/dcodeIO/ProtoBuf.js/wiki/proto2js. From what I can see in the documentation, you'd usually not compile the .proto files into .js files, but just load the .proto files at runtime using the protobuf.js library.

Compiling the .js files is also a bit tricky, because it depends on the environment how dependencies should be loaded. nodejs uses commonjs for dependencies. The browser could use AMD/RequireJS, commonjs, or whatever else people came up with. There isn't one way to do this, so I'm leaning towards dropping JS-file generation.

zultron commented 9 years ago

On 08/29/2015 08:07 AM, Bob van der Linden wrote:

I wasn't entirely sure where the dependency files (|.d|) were used for. Since I couldn't generate those earlier I thought they were meant to be d source code files (the D programming language). What are they exactly used for?

Those are dependency files. They're sourced in the Makefile, and help make figure out when to rebuild after a header file changes.

bobvanderlinden commented 8 years ago

Ah, nice. This wasn't working correctly in my version, but that problem is now fixed.

On that topic, could you take a look at #23? Generating the .d files wasn't working on my machine because of a text<->bytes problem. It is a different problem than the namespacing, so I created a new PR for that.

Also, I've added a install rule that'll basically copy the .proto files to /usr/local/include (or whatever DESTDIR is set to). It'll also install the .h files, but does not yet compile the .c files into a .so/.a file (and so does not yet install the .so/.a). I have not yet changed the package names (package pb; to package machinetalk.protobuf;). Let me know if this is indeed the way to go, or that there are still problems with nanopb?

Lastly, I've started on a node API for machinetalk. It was much easier to set this up compared to Python, because all required libraries (mdns/zeroconf, zmq and protobuf) were already there. (In Python I had the problem that the libraries weren't cooperating correctly). It does discovery, it connects and it reads out full and incremental task-status updates. The repository can be found here: https://github.com/bobvanderlinden/node-machinetalk

mhaberler commented 8 years ago

what ?! one day later and you're over the hurdles ;-?

great!

bobvanderlinden commented 8 years ago

I feel like this issue covers a number of issues, so I have split this issue into #27 (package namespacing), #24 (make install), #25 (NodeJS package) and #26 (Python package). I'll close this one so that discussion can focus on a single issue.