-
Notifications
You must be signed in to change notification settings - Fork 28
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
Merge esphome-core into esphome #97
Comments
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. |
## Description: Move esphome-core codebase into esphome (and a bunch of other refactors). See esphome/feature-requests#97 Yes this is a shit ton of work and no there's no way to automate it :( But it will be worth it 👍 Progress: - Core support (file copy etc): 80% - Base Abstractions (light, switch): ~50% - Integrations: ~10% - Working? Yes, (but only with ported components). Other refactors: - Moves all codegen related stuff into a single class: `esphome.codegen` (imported as `cg`) - Rework coroutine syntax - Move from `component/platform.py` to `domain/component.py` structure as with HA - Move all defaults out of C++ and into config validation. - Remove `make_...` helpers from Application class. Reason: Merge conflicts with every single new integration. - Pointer Variables are stored globally instead of locally in setup(). Reason: stack size limit. Future work: - Rework const.py - Move all `CONF_...` into a conf class (usage `conf.UPDATE_INTERVAL` vs `CONF_UPDATE_INTERVAL`). Reason: Less convoluted import block - Enable loading from `custom_components` folder. **Related issue (if applicable):** esphome/feature-requests#97 **Pull request in [esphome-docs](https://github.com/esphome/esphome-docs) with documentation (if applicable):** esphome/esphome-docs#<esphome-docs PR number goes here> ## Checklist: - [ ] The code change is tested and works locally. - [ ] Tests have been added to verify that the new code works (under `tests/` folder). If user exposed functionality or configuration variables are added/changed: - [ ] Documentation added/updated in [esphomedocs](https://github.com/OttoWinter/esphomedocs).
Done. Unfortunately, all existing PRs are invalid because of this, but I think the improved API/development process will be worth it. |
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:
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
:Sample
dht_component.cpp
:Sample
dht/sensor.py
On build, all source files are copied into the src folder.
Improvements:
This will have a number of improvements:
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
This would obviously be a huge refactor, but I think it would be very useful and be an important milestone to make contributing easier.
The text was updated successfully, but these errors were encountered: