PelionIoT / mbed-trace

mbed trace library
Apache License 2.0
18 stars 15 forks source link

Add a simple makefile for nanomesh-libservice integration. #22

Closed tommikas closed 8 years ago

tommikas commented 8 years ago

@jupe @SeppoTakalo

This would also benefit from ARMmbed/mbed-trace#20, which should be ready to be merged.

jupe commented 8 years ago

I don't like idea to have this kind of Makefile which related totally different module, even this help "libservice integration".. This cannot be only way to do this, or is it @SeppoTakalo @kjbracey-arm @tommikas ?

tommikas commented 8 years ago

Having it inside the module itself is consistent with how it's done with all the other exported libraries, which is the reason I did it like this. (The ones here: https://github.com/ARMmbed/nanomesh-libservice-root-private/tree/master/exported-libs)

Technically it would of course be possible to have it pretty much anywhere. I suppose there was a logic to the placement of the makefiles in the previously added libraries? @jupe @SeppoTakalo @kjbracey-arm

jupe commented 8 years ago

this might causes problem&confusion when we create makefile like this which compile only this module, like yotta does for now..

tommikas commented 8 years ago

It occurs to me that this makefile is also not quite equivalent to those of the other exported-libs libraries' makefiles, since they include the actual build rules from the previous level. Would it suffice if the makefile in this module was equally generic? That would make it possible to include this module in any application with application specific build rules defined outside the module.

There's plenty of ways to avoid/clear that kind of confusion anyway though. The nanomesh-libservice specific makefile could be called something other than "Makefile", like Makefile.nanomesh or whatever, or a recipe that is specific to this use could be defined, or a variable could be checked for.

In any case, I'm not married to the idea of a Makefile in mbed-trace module itself. It seemed the easiest way to retain the yotta style include path ("#include mbed-trace/mbed_trace.h") that the applications that use mbed-trace expect, which is something the included generic make rules right now wouldn't do. Happy to do whatever is considered best.

kjbracey commented 8 years ago

Libservice is a sort of bundling mechanism - it's supposed to be a single "base" library for Nanostack and related components.

Various bits of libservice have previously been pulled out to other repositories (for Yotta), and as that's been done, the hooks have been left in place such that building libservice continues to provide all the same facilities, even though the code now lives in different libraries.

But on the other hand, as it has a different library name, and the init API is different, the bundling doesn't make the transition totally seemless - apps will still need tweaking. But nevertheless:

Libservice is supposed to be the base library. If any code in Libservice calls trace, then the trace needs to be in libservice too. I don't want a lower-level-than-libservice library.

So I'm conceptually happy with this makefile going in.

I agree that it should probably use exported_rules.mk like the others if possible.

kjbracey commented 8 years ago

On the naming point, which I missed - currently all these exported libraries have no Makefile other than the "Nanostack" one. Yotta-type tools ignore the Makefile. Maybe "Makefile.nanostack" leaves room for some other standalone Makefile, but we've not needed that yet, and it would be inconsistent with other ex-libservice libraries. But I'd be okay with it.

jupe commented 8 years ago

Makefile.nanostack, or should it be Makefile.libService because its used from libService, or is it? sounds good for me. at least it leaves room for other solutions and we doesnt need to think this soon again..

tommikas commented 8 years ago

Renamed Makefile to Makefile.nanomesh as it is there to support the nanomesh build environment.

SeppoTakalo commented 8 years ago

+1

kjbracey commented 8 years ago

Fine, but squash all this together.