machinekit / machinetalk-protobuf

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

Namespace might conflict with other packages #27

Closed bobvanderlinden closed 8 years ago

bobvanderlinden commented 8 years ago

The namespace pb might conflict with other packages as it is too generic. Protobuf uses a namespace that is more descriptive and unique (google.protobuf). When we want to install the protobuf files into the right directories, we need to make sure it doesn't overlap with other packages.

I do not know what the consequences are for nanopb, but I think this is the right place to discuss that.

I think this needs to be done before we roll out separate packages as described in #24, #25 and #26.

mhaberler commented 8 years ago

makes sense, so let's go through the shopping list:

C++ fallout (1) - dereferenced names: `grep -r namespace .|grep 'pb;'

./machinetalk/support/encdec.cc:using namespace pb; ./machinetalk/support/position.cc:using namespace pb;

easy, I can take that - should go for fully qualified names in C++ from now on

C++ fallout (2) - fully qualified names $ grep -lr pb::Container .|wc -l 35

will take that, single emacs mass reedit

nanopb C fallout - not much, can take that

Since the wireformat doesnt change, that change is local to the machinekit repo

@strahlex - what would be involved on your side to make QtQuick VCP build from a mutated machinetalk-protobuf branch? will your source be backwards compatible or is that forward only?

I think step 1 would be to temporarily introduce a branch in this repo which has the new namespace conventions, then merge that into machinekit as a subtree - that enables fixing the mk repo

independently Alex can have a got at it, until then keeps using the current branch

depending on results we might have to do a synced upgrade, or a independent one

machinekoder commented 8 years ago

@mhaberler The code should be pretty much compatible since I do not use the Makefile to generate my protobuf classes anyway.

I think we can go forward on this topic as it simplifies using Machinetalk on non-Machinekit computers significantly: https://github.com/machinekit/machinetalk-protobuf/pull/36

machinekoder commented 8 years ago

Has been merged.