espressif / esp-idf-cxx

C++ wrapper classes for ESP-IDF components.
Apache License 2.0
79 stars 14 forks source link

release plan? #12

Open ogghst opened 2 years ago

ogghst commented 2 years ago

wow, this sounds like a breaking change for esp-idf developers! is there a kind of plan for a stable and comprehensive release?

0xjakob commented 2 years ago

Hi @ogghst

wow, this sounds like a breaking change for esp-idf developers!

Not at all. This will not break any existing code based on ESP-IDF (unless you actually used the code in the experimental C++ component of IDF, which is moved to this repository).

We have released the beta version of the component for the component manager already. We'll probably have changes in the future, but haven't decided on a release cycle/plan/policy yet. In general, you can expect it to be roughly similar to ESP-IDF.

What we definitely plan is to add C++ code for:

ogghst commented 2 years ago

Thanks! I was already assuming C would have kept as main language, but your explanation is now clarifying me what is the final plan.

finger563 commented 2 years ago

@0xjakob Are there plans for adding support for SPI devices which do not return data and therefore have no MISO line, such as display controllers? Right now the SPIMaster component does not support these types of devices because:

  1. idf::MISO cannot be provided a -1 value, or some other type of "don't care" value
  2. device->transfer(...) asserts if the device does not return data.

Would you be open to a PR adding these features?

0xjakob commented 2 years ago

@finger563 Honestly, I never really tried a device without MISO line with esp-idf-cxx. I thought it would just work by chosing a random pin. If it doesn't, it's definitely a missing feature. I also have a few e-inks with this setup lying around.

PRs are very welcome! Usually, we are trying to avoid breaking changes, but I'm not sure if this is possible as I suspect the idf::SPIFuture class might need some changes to accommodate different return types. On the other hand, maybe it's still possible to make the change non-breaking. Furthermore, this project is still in a beta-phase. You can find some further guidelines in CONTRIBUTING.md. Please let us know if you have more questions; any organizational or design questions shouldn't stop you from contributing!

BTW, if you have a different topic as the one from the actual issue topic, feel free to open a new issue. This helps us keeping track of new questions.

sukesh-ak commented 1 year ago

@0xjakob Any plans to pull the esp_mqtt_cxx from idf tree? https://github.com/espressif/esp-idf/tree/master/examples/cxx/experimental/esp_mqtt_cxx

0xjakob commented 1 year ago

@sukesh-ak The engineers working on C++ MQTT code plan to add it to another component that can be used with the component manager.

sukesh-ak commented 1 year ago

@sukesh-ak The engineers working on C++ MQTT code plan to add it to another component that can be used with the component manager.

Is there a repo I can monitor for it?

0xjakob commented 1 year ago

@sukesh-ak

Is there a repo I can monitor for it?

Probably https://github.com/espressif/esp-mqtt

0xjakob commented 1 year ago

@sukesh-ak Please check https://github.com/espressif/esp-protocols/pull/189 for the future location of C++ MQTT.

jdoubleu commented 1 year ago

Are contributions welcome? I have a bunch of C++ classes written over the time, wrapping some ESP-IDF functions and concepts.

But more importantly, what's your goal with this library? How do you want to approach the library design. What I've seen in the event api is that you just wrap the C functions (e.g. esp_event_handler_instance_register and simply forward all paramters). But C++ offers so much more, especially with RAII and other powerful idioms.

From my experience, it's not always best to just wrap the ESP-IDF C functions. Take the event API for example: the C++ classes may call the C API directly and just hold the esp_event_handler_t handle. The purpose of the wrapper would be to own the handle and properly manage its lifetime (i.e. calling unregister on destruction). However, now both the C library and the C++ wrapper manage the same event handler. The C library keeps a list internally of each registered handler. This not only adds an extra hoop for C++ to go through, but may also complicated the wrapping of some concepts. \ What I thought about was re-implementing these components specifically for C++, written in C++ using modern containers, etc.

igrr commented 1 year ago

What I thought about was re-implementing these components specifically for C++, written in C++ using modern containers, etc.

One possible downside to consider in this approach is that it will cause larger binary size and less heap memory available:

0xjakob commented 1 year ago

@jdoubleu

Are contributions welcome?

Contributions are always welcome.

But more importantly, what's your goal with this library?

The goal of this library is to use C++ to make IDF APIs as easy-to-use as possible, while remaining applicable to "most" (let's say, 95%) use cases. A particular focus is people who are not so familiar with embedded devices but are familiar with concepts like object-oriented programming and exceptions (exceptions as part of the programming language, not CPU exceptions!). Another focus is people who don't want to type and think too much to trigger a GPIO or read an I2C sensor. This library should furthermore give a good mixture of using default values (many of which can still be changed) and configuration-via-type (see GPIO_Output class) to make programming as easy as possible.

hat I've seen in the event api is that you just wrap the C functions (e.g. esp_event_handler_instance_register and simply forward all paramters). But C++ offers so much more, especially with RAII and other powerful idioms.

Maybe there is a misunderstanding here, but as I see it, the entire library is built on RAII. There are few places where you actually need some static (de)initialization functions. Furthermore, we use strong-value types to prevent confusion of parameters in many places, just as an example of C++ idiom that makes programming safer.

If you don't mind, would you like to point out the places where you'd expect more RAII usage or other C++ idioms?

jdoubleu commented 1 year ago

Maybe there is a misunderstanding here, but as I see it, the entire library is built on RAII.

My fault, I haven't look at the esp_event_cxx.hpp. That covers most of what I'm looking for. Although the interface is not exactly what I think best fits my requirements. There are a bunch of heap allocations I'd like to avoid (take the std::unique_ptr<ESPEventReg> for example). Additionally, most of the parameters are known at compile time (i.e. event_base and event_id). I don't want to carry them around inside a ESPEvent struct.

One thing that would be really nice to have, is to support type-save event handlers. Instead using an event callback which accepts void* event_data (e.g. std::function<void(const ESPEvent &, void*)), depending on the event, the callback would accept the actual data. For the WIFI_EVENT_STA_CONNECTED event that's a wifi_event_sta_connected_t* for example.

However, that requires some manual effort. It may be accomplished through traits and would best work with templates (of EventReg), I think. I'm thinking of something like:

template<esp_event_base_t EventBase>
struct event_traits;

template<>
struct event_traits<WIFI_EVENT>
{
   template<int32_t EventID>
   struct event;

   template<>
   struct event<WIFI_EVENT_STA_CONNECTED>
   {
      using event_data = wifi_event_sta_connected_t;
   };
};

template<esp_event_base_t EventBase, int32_t EventID>
class ESPEventReg
{
  // ...
  std::unique_ptr<ESPEventReg> register_event(
            std::function<void(event_traits<EventBase>::event<EventID>::event_data)> cb);
}

In my project, I've written a bunch of minimal RAII wrappers for global modules like nvs, netif, mdns, etc.

These are just some ideas I had.