esphome/feature-requests

Merge esphome-core into esphome

OttoWinter opened this issue · 4 comments

Merge all C++ files into the esphome repository so that the python and C++ files live right next to each other.

This is only an early idea I had the other day, and I haven't thought it through 100%. Just wanted to write it down somewhere and potentially receive some feedback.

The Problem(s)

There are a number of problems in esphome that could all be solved by a major refactor:

  • Compile Time: It can take quite a while to compile a simple esphome project, and each time the user adds a component the whole project needs to be rebuilt. This will only get worse over time with new components.

  • Contributing: Contributing to ESPHome is not that nice, PRs need to be made against 3 repos: esphome, esphome-core and esphome-docs.

  • Viewing Source: Often you want to view the C++ source together with the python source, this is very difficult atm.

  • Custom Components: Creating true custom components (that have python c++ generators too) is not possible. So if someone develops a custom component that doesn't fit into esphome officially (licensing issues, etc) you need to maintain a fork

  • Core and ESPHome versions are out of sync: This bugs me a lot. a) it breaks everyone who attempts to use esphome_core_version: dev and b) when submitting a PR, the python test cases will fail until the core PR has been merged.

  • Defaults are all over the place: Default values for integration parameters are all over the place in different places: some in the integration header file, some in application.h, and some in python validation.

The proposal

Merge esphome-core into esphome

What does that mean? All C++ files will be moved into the esphome repository. I think an example explains this best:
This would be the directory layout for the esphome repo:

esphome
├── core
│   # Core files used by all components; to glue everything together
│   ├── component.cpp
│   ├── component.h
│   ├── esphal.h
│   └── helpers.h
├── components
│   ├── dht
│   │   # Each integration has its own directory
│   │   ├── dht_component.cpp
│   │   ├── dht_component.h
│   │   # This will be called by `sensor: - platform: dht` (see the great HA migration)
│   │   ├── sensor.py
│   │   # Empty file to mark this as a python module (until py3)
│   │   └── __init__.py
│   └── sensor
│       ├── __init__.py
│       ├── filter.cpp
│       ├── filter.h
│       ├── sensor.cpp
│       └── sensor.h
├── __init__.py
├── __main__.py
├── cpp_generator.py
└── dashboard
setup.py
platformio.ini
.travis.yml

As you can see, C++ and python source files are in the same repository, and all source files for a component are
in one directory.

Sample dht_component.h:

#ifndef ESPHOME_DHT_COMPONENT_H
#define ESPHOME_DHT_COMPONENT_H

// note: no more #ifdef USE_DHT_SENSOR

#include "esphome/components/sensor/sensor.h"

namespace esphome {

// note: no more 'namespace sensor {'
class DHTComponent : public PollingComponent {
  // ...
};

}  // namespace esphome

#endif // ESPHOME_DHT_COMPONENT_H

Sample dht_component.cpp:

// note: no more #ifdef USE_DHT_SENSOR
#include "esphome/components/dht/dht_component.h"
#include "esphome/core/log.h"
#include "esphome/core/helpers.h"

namespace esphome {

static const char *TAG = "sensor.dht";

// ...

}  // namespace esphome

Sample dht/sensor.py

DHTComponent = esphome_ns.class_('DHTComponent', PollingComponent)
PLATFORM_SCHEMA = sensor.PLATFORM_SCHEMA.extend({...}))

def to_code(config):
    for pin in gpio_output_pin_expression(config[CONF_PIN]):
        yield
    # Note: No more App.make_dht_sensor
    rhs = DHTComponent.new(...)
    # ...

# Note: No more BUILD_FLAGS
# source files to copy, relative to python file, we could potentially also automate this
SOURCE_FILES = ['dht_component.h', 'dht_component.cpp']

On build, all source files are copied into the src folder.

Improvements:

This will have a number of improvements:

  • Compile Time:

    • We will not have to update global compiler flags when a component is added/removed, we only need to copy/remove files. This means we don't have to recompile.
    • If you don't use component X, it also won't be compiled. Previously the .cpp file was compiled- it was not in the binary because of the #ifdef USE_x but contributed to the compile time.
  • Contributing: Only one PR needs to be made for code changes. (another one is still needed for docs - i won't merge the two though because esphome-docs is quite big with all image assets)

  • Viewing Source: As each component has its own dir, all sources are directly accessible.

  • Custom Components: Would be possible, with the exact mechanism like HA

  • Core and ESPHome out of sync: Obviously no issue anymore

  • Inter-Component Dependencies: This will additionally enforce the barrier between components. Each component will only be able to use core/ code or ones that are explicitly imported in the python file.

Changes Needed

  • All central references to components need to be removed. A component should be 100% self-contained in the directory it is in. The Application App instance is a problem here. So application (which is only a tiny setup helper) will be replaced by direct calls to the component constructor (see dht example above).

  • Some components like esp32_ble_tracker have platforms that do not share the name of the component: xiaomi_mijia, xiaomi_miflora, esp32_rssi etc. This breaks the directory structure a bit - probably the workaround will be a manual override in the component loader (ugly but works).

  • I want to drop all namespaces in this project except for esphome. For example, users had to type cover::COVER_OPEN before, now that would only be COVER_OPEN. namespaces are good for avoiding name conflicts, but I don't think I've ever encountered one yet except for sensor/binary_sensor filters (which are easily renamed).

  • Probably more, will update this post.

Potential Problems

  • Build System: This will only work if it works well with IDEs. Platformio needs to be able to treat the esphome folder as a C++ project. If I don't get platformio to work well together with this new directory structure I will not do this. I need smart syntax highliting and actions in C++ :D
  • Licensing: The ESPHome-Core is licensed under GPLv3, the python code under MIT. Either we relicense everything or maintain the old one with a split license (currently tending to the latter)

This would obviously be a huge refactor, but I think it would be very useful and be an important milestone to make contributing easier.

Interesting. Please avoid use of symbolic links 👍

I think this would improve the ux greatly, especially for those poor souls compiling on a SOC. It may help you going forward too, as adding a new component would require only a single code change, it may make it easier for you to get some help :)

I like the idea, I agree it would help make the development process a bit easier.

Done. Unfortunately, all existing PRs are invalid because of this, but I think the improved API/development process will be worth it.