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
namespace
s in this project except for esphome. For example, users had to typecover::COVER_OPEN
before, now that would only beCOVER_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.