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

C++ support and MQTT #17

Open
mikejac opened this issue Jul 30, 2015 · 11 comments
Open

C++ support and MQTT #17

mikejac opened this issue Jul 30, 2015 · 11 comments

Comments

@mikejac
Copy link
Contributor

mikejac commented Jul 30, 2015

This it not an issue as such :-)

  1. How do you feel about having the ability to use C++ within this project? I have forked esp-open-rtos (https://github.com/mikejac/esp-open-rtos) and made some initial steps to get C++ supported. However, I'm in no way an Makefile/make expert, so would you take a look at the /common.mk file (lines 183 - 187 I believe) and perhaps think of a way to get .cpp files compiled with their own compiler flags (CXXFLAGS).

  2. How about adding the Eclipse Paho MQTT client library to esp-open-rtos? Already got it "ported" locally.

@kanflo
Copy link
Contributor

kanflo commented Jul 31, 2015

I wouldn't mind C++ support. Do global constructors get called automagically? I saw someone else add C++ support and included code for this.

Adding MQTT sounds great. @projectgus is working on adding an "extras" or "components" directory where a project can select modules/drivers to be included. When that work is done, an addition of MQTT would fit nicely.

@mikejac
Copy link
Contributor Author

mikejac commented Jul 31, 2015

AFAIK, no, global constructors does not get called. @jcmvbkbc had some ideas here: http://www.esp8266.com/viewtopic.php?f=9&t=478&sid=e39885fb994d80e87a2fb24c56e58e8f&start=5.

I've used that technique "outside" esp-open-rtos. There seem to be something with overridden virtual functions that doesn't work 100% - needs more testing.

@projectgus
Copy link
Contributor

Sorry @mikejac, I thought I replied to this post on Friday but it seems my reply got eaten somewhere. :(

AFAIK, no, global constructors does not get called

Most of the building blocks for open sourcing the startup code (#2) are in place, so once we have that it will be possible to call global constructors properly at the right place in the startup sequence, and everything should "just work". At the moment the best solution is probably calling them manually from user_start, as mentioned.

I'm in no way an Makefile/make expert, so would you take a look at the /common.mk file (lines 183 - 187 I believe) and perhaps think of a way to get .cpp files compiled with their own compiler flags (CXXFLAGS).

Yeah, common.mk (especially the non-recursive-make dynamic generation parts) gets pretty $$($(nasty))!

In 30877f4 (ota branch) I recently added support for assembling .s files. C++ support will probaby look similar, you may need to generate a $$($(1)_CXX_ARGS) with the C++ compiler flags though (you can probably model this on $(1)_CC_ARGS. Does that help?

If you get stuck then I don't mind helping to finish it off, the stuff you've done already is 99% of the work! I probably won't have time until after the ota branch is merged, though (should be next week). :)

  1. How about adding the Eclipse Paho MQTT client library to esp-open-rtos? Already got it "ported" locally.

Yes please, that'd be fantastic! As @kanflo mentioned, the plan is to have an 'extras' directory with components that aren't added by default, that can be compiled in if needed.

That's also implemented in the ota branch already, take a look at 2440ba5 and 147257e, particularly extras/rboot-ota/component.mk & examples/ota_basic/Makefile

Ideally I'd like to follow a similar pattern to lwip & axtls, where the "upstream" repo is added as a submodule in a subdirectory (for easy tracking of updates) and then any ESP-specific files sit in the parent directory.

@projectgus projectgus mentioned this issue Aug 10, 2015
@mikejac
Copy link
Contributor Author

mikejac commented Aug 18, 2015

I'm getting ready to publish some C++ code - which include some simple wrapper classes for FreeRTOS and an MQTT client (based on the Paho embedded C++ client).

I'll be back :-)

@mikejac
Copy link
Contributor Author

mikejac commented Aug 20, 2015

  • Added some 'extra' header-files and a simple task/queue example to my fork.
  • Added '-fno-rtti' flag to the CXXFLAGS. If not present, code with virtual functions do not compile.
  • Added '$(ROOT)extras' to INC_DIRS in order to include headers like this: #include "thread/task.hpp" where 'thread' is a directory in 'extras'.

Please have a look and let me know what you think.

@projectgus
Copy link
Contributor

Hi Mike,

This is great, very excited that you have MQTT working and C++ native wrappers for common functions.

I have a few general comments, mostly about code organisation:

  • In common.mk, it shouldn't be necessary to add $(ROOT)extras to the INC_DIRS variable globally. The INC_DIRS variable should only need modifying if a component is added from under the 'extras' dir, and it should be modified from inside the component.mk makefile (ie INC_DIRS += extras/blah.)
  • What would you think about combining the C++ wrapper classes (extras/timer,extras/wifi, extras/thread) into a single component, something like 'extras/cpp_support'? Which can have a single include directory extras/cpp_support/include which contains all the hpp files, and then put all the individual C++ files into a separate directory. That way if someone wants to use C++ wrappers they can just add a single component to the build, and pick and choose whatever header files they need from there.
  • extras/paho-mqtt/MQTTPacket/src/component.mk should ideally be in the top-level directory I think, ie extras/paho-mqtt/component.mk. Unless I'm missing something about the way paho-mqtt works.
  • Can you elaborate at all on where extras/ip/ipstack.hpp comes from and what it's for? Is it an abstraction layer for Eclipse Paho, so it works with LWIP? Could it be merged into another component, maybe extras/cpp_support or extras/paho-mqtt?
  • Did the rest of paho-mqtt require any source code modifications? Would it be possible to include it as a git submodule pointing to the Paho upstream, instead of directly in the code, or does it require patches?
  • The "Barracuda" stuff in extras/json doesn't appear to be under an OSI license, so I can't merge it. There are a few other embedded-friendly JSON libraries out there though, I think.

Thanks again for all this, it's going to be terrific to offer this level of C++ support!

@mikejac
Copy link
Contributor Author

mikejac commented Aug 21, 2015

In common.mk, it shouldn't be necessary to add $(ROOT)extras to the INC_DIRS variable globally. The INC_DIRS variable should only need modifying if a component is added from under the 'extras' dir, and it should be modified from inside the component.mk makefile (ie INC_DIRS += extras/blah.)

Corrected (more fun with make).

What would you think about combining the C++ wrapper classes (extras/timer,extras/wifi, extras/thread) into a single component, something like 'extras/cpp_support'? Which can have a single include directory extras/cpp_support/include which contains all the hpp files, and then put all the individual C++ files into a separate directory. That way if someone wants to use C++ wrappers they can just add a single component to the build, and pick and choose whatever header files they need from there.

Fine with me. Take a look and let me know what you think (Don't pull it just yet - want to make some minor adjustments).

extras/paho-mqtt/MQTTPacket/src/component.mk should ideally be in the top-level directory I think, ie extras/paho-mqtt/component.mk. Unless I'm missing something about the way paho-mqtt works.

I've kept it that way as that is the layout from the original source. Think it will make it easier to compare with the original as well as updating it in the future if the directory layout is kept as in the original.

Can you elaborate at all on where extras/ip/ipstack.hpp comes from and what it's for? Is it an abstraction layer for Eclipse Paho, so it works with LWIP? Could it be merged into another component, maybe extras/cpp_support or extras/paho-mqtt?

ipstack_t (in the 'ip' directory) and countdown_t are both used by the C++ MQTTClient. And yes, it's an abstraction layer. So it's used by MQTTClient which in turn is used by mqtt_client_t. And maybe someone could use it for something else. So where to put it? :-)

Did the rest of paho-mqtt require any source code modifications? Would it be possible to include it as a git submodule pointing to the Paho upstream, instead of directly in the code, or does it require patches?

Only MQTTClient has been modified (MQTTClient.h and FP.h). The code in the MQTTPacket directory has not been modified - and that code can be used by C as well as C++.

The "Barracuda" stuff in extras/json doesn't appear to be under an OSI license, so I can't merge it. There are a few other embedded-friendly JSON libraries out there though, I think.

Thought so, just didn't get around to ask you guys about their license. This particular JSON library does have something going for it:

  • Works with C and C++.
  • Constructed for use on memory-constrained systems (you can define your own alloc/free functions).
  • Supports streaming input, i.e. you can add one byte at time to the decoder, and it will tell you if the JSON struct is complete, if there was an error or if it needs more data. Good for inputting serial data.

@projectgus
Copy link
Contributor

Hi @mikejac,

I've merged the extras/cpp_support stuff with a couple of tweaks and pushed them to #24.

I'm leaving the MQTT stuff separate for now. I have it in a branch but I want to do a small rearrange and then I'll put it into another pull request for you to take a look at.

Thought so, just didn't get around to ask you guys about their license. This particular JSON library does have something going for it
snip

I agree it looks good technically. I originally just saw that the headers that said "Barracuda Application Server", which is a proprietary paid library, and assumed there was no possible way it could be merged into an OSS project!

However with a bit more digging I found this page and realised there was originally a README saying these parts are under the Eclipse Public License. So I think it's probably fine to merge as an optional 'extra', we just need to include the fact that we're taking it from an Eclipse Public License release not proprietary software!

Anyhow, as I said I have the basics in #24. I'm happy to do the merge work for MQTT soon as well, will put up a PR for review once done.

I have a couple of minor tips regarding git usage as well: your git email is currently not resolving to your github username, so your commits aren't associated with you. You can use 'git config --global user.email me@my.com' to set this to the email you want to use. One other thing, 'squashing commits' is an invaluable thing to look into. It's something you can do before pushing branches - cleans up history before you send it so things look nicer. :)

Thanks again for all this!

@Angus71
Copy link
Contributor

Angus71 commented Aug 28, 2015

Hi @mikejac, @projectgus,

It's always good to read "all" issues :-)

I spend a "rainy" afternoon to port the above mentioned paho-mqtt client (The Embedded-C version) and not the C++ version) to work with esp-open-rtos.

Is this still of any interest? I think it might not be a problem to support both approaches (C & C++).
If this is of any interest, can can push the changes into my repository for review.
Even if this is not of any interest, I still learned something and the time isn't lost.

EDIT: I added the mqtt embeded stuff into my repository (Angus71/esp-open-rtos@b557f4f), but I didn't found a way to modify the extras/mqtt_embed_c/mqtt/MQTTClient-C/src/MQTTClient.h file from the paho project, as it requires a slight modification (#include ""needs to be modified to #include "MQTTCFreeRTOS.h"), to reference the FreeRTOS support files. I didn't want to mirror the paho project in an own repository, just for modifying one #include. I tried several things, but the main problem is, that while compiling always -I ./ is part of the compiler switches and by that the MQTTClient.h from the paho repository gets pulled in, which is not compiling, due to the empty #include.
Does anybody have an idea how to "keep" the external repository and use a "MQTTClient.h" file from a different folder?
Sorry for being a little bit off-topic, but I think this might also be useful for the C++ version, because it may have the same problem.

Regards
Frank

@projectgus
Copy link
Contributor

C++ support in #24 has been merged.

It's taken me a bit longer to get to the Paho MQTT client stuff than I hoped, but it's still on my list!

@Angus71, sorry I missed your follow-up question (github doesn't send emails when you edit posts, I don't think.) I am very interested in having the C client and the C++ client. I'll try and make time to pull down your changes and look at the header problem, soon.

@sarwadenj
Copy link

sarwadenj commented Oct 24, 2016

Hi @mikejac,@projectgus
I'm new to esp-open-rtos and want to use c++ coding. So for that i don't know how i add c++ support (how i make setup) to esp-open-rtos.
for that i have to install c++ compiler also i have to make changes in make changes in make file and common.mk file So please guide me how i do that

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

No branches or pull requests

5 participants