mirror of
https://github.com/esphome/esphome.git
synced 2026-09-04 03:56:04 +00:00
Compare commits
13
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
421c5e5d32 | ||
|
|
ab59822c1f | ||
|
|
fa5180bd51 | ||
|
|
8582bf194a | ||
|
|
328e83ad64 | ||
|
|
6877513195 | ||
|
|
03104b894b | ||
|
|
26a308a82d | ||
|
|
938f598709 | ||
|
|
2b4a196cf8 | ||
|
|
3f65f5c12c | ||
|
|
7cc892ea14 | ||
|
|
9e4bec7e49 |
@@ -44,16 +44,6 @@ This document provides essential context for AI models interacting with this pro
|
||||
|
||||
## 4. Coding Conventions & Style Guide
|
||||
|
||||
**Read the developer documentation before writing a component.** https://developers.esphome.io covers the
|
||||
component lifecycle, the main loop, and the reasoning behind the rules below in far more depth than this
|
||||
file does, and it is the authority when they disagree. The most useful starting points:
|
||||
|
||||
* https://developers.esphome.io/architecture/components/ - component lifecycle, `setup()`, `loop()`,
|
||||
setup priorities, and how a component is registered.
|
||||
* https://developers.esphome.io/architecture/components/advanced/ - choosing between `loop()`,
|
||||
`set_interval`, `set_timeout` and `defer`; waking the loop from another thread; the RAM cost of each.
|
||||
* https://developers.esphome.io/contributing/code/ - contribution rules, public API and breaking changes.
|
||||
|
||||
* **Formatting:**
|
||||
* **Python:** Uses `ruff` and `flake8` for linting and formatting. Configuration is in `pyproject.toml`.
|
||||
* **C++:** Uses `clang-format` for formatting. Configuration is in `.clang-format`.
|
||||
@@ -152,47 +142,6 @@ file does, and it is the authority when they disagree. The most useful starting
|
||||
* **Indentation:** Use spaces (two per indentation level), not tabs
|
||||
* **Type aliases:** Prefer `using type_t = int;` over `typedef int type_t;`
|
||||
* **Line length:** Wrap lines at no more than 120 characters
|
||||
* **Timing in `loop()`:** Never call `millis()` in a `loop()` body. The current tick's timestamp is
|
||||
already cached - use `App.get_loop_component_start_time()` (from `esphome/core/application.h`).
|
||||
Only reach for `millis()` when you genuinely need sub-tick resolution inside a long operation.
|
||||
* **The main loop runs every 16 ms.** A rate-limit gate shorter than that does nothing: the check
|
||||
passes on essentially every pass of the loop, so it costs a comparison and buys nothing. Pick an
|
||||
interval comfortably coarser than 16 ms, or drop the gate entirely and accept running every loop.
|
||||
```cpp
|
||||
// Bad - a 10ms gate against a 16ms loop never holds anything back
|
||||
static constexpr uint32_t POLL_INTERVAL_MS = 10;
|
||||
const uint32_t now = millis();
|
||||
if (now - this->last_poll_ < POLL_INTERVAL_MS)
|
||||
return;
|
||||
this->last_poll_ = now;
|
||||
```
|
||||
```cpp
|
||||
// Good - an interval that actually rate limits, off the cached timestamp
|
||||
static constexpr uint32_t POLL_INTERVAL_MS = 100;
|
||||
const uint32_t now = App.get_loop_component_start_time();
|
||||
if (now - this->last_poll_ < POLL_INTERVAL_MS)
|
||||
return;
|
||||
this->last_poll_ = now;
|
||||
```
|
||||
Pick the primitive by cadence: under 250 ms use a gated `loop()`; 500 ms and above use
|
||||
`set_interval`. Full reasoning, including why `set_interval` costs more below 500 ms:
|
||||
https://developers.esphome.io/architecture/components/advanced/#quick-rule-of-thumb
|
||||
* **Don't override a default with the same value:** if a base class method already returns what you
|
||||
want, do not override it. `Component::get_setup_priority()` returns `setup_priority::DATA`, so a
|
||||
component that wants `DATA` should simply leave it alone.
|
||||
```cpp
|
||||
// Bad - this is exactly what the base class already does
|
||||
float get_setup_priority() const override { return setup_priority::DATA; }
|
||||
```
|
||||
* **Logging string literals:** wrap literals passed as `%s` arguments in `LOG_STR_LITERAL()` so they
|
||||
can be stored in flash rather than RAM.
|
||||
```cpp
|
||||
// Bad
|
||||
ESP_LOGV(TAG, "Key %u %s", key, pressed ? "pressed" : "released");
|
||||
|
||||
// Good
|
||||
ESP_LOGV(TAG, "Key %u %s", key, pressed ? LOG_STR_LITERAL("pressed") : LOG_STR_LITERAL("released"));
|
||||
```
|
||||
* **Constructor parameters vs setters:** Component properties that are both **required** and **invariant**
|
||||
(never change after construction) should be constructor parameters rather than set via setter methods.
|
||||
This makes the dependency explicit and prevents use of the object in an incompletely-initialized state.
|
||||
@@ -613,33 +562,6 @@ file does, and it is the authority when they disagree. The most useful starting
|
||||
Use `cg.add_define("MAX_SERVICES", count)` to set the size from Python configuration.
|
||||
Like `std::array` but with vector-like API (`push_back()`, `size()`) and no STL reallocation code.
|
||||
|
||||
**Listener and child-entity registration lists are the most common case, and the most commonly
|
||||
missed.** A `register_*()` method called once per child at code generation time has a count that
|
||||
is known at compile time, so it should never be a `std::vector`. Use `cg.slot_counter()`: it
|
||||
returns a function that each consumer calls once per slot it will occupy, and after every
|
||||
`to_code` has run it emits the define with the final count. When nothing registers, no define is
|
||||
emitted and the storage plus its registration method compile out entirely.
|
||||
```python
|
||||
# hub component's __init__.py
|
||||
_request_listener_slot = cg.slot_counter("MY_COMPONENT_LISTENER_COUNT")
|
||||
|
||||
|
||||
async def register_listener(hub: MockObj, var: MockObj) -> None:
|
||||
_request_listener_slot()
|
||||
cg.add(hub.register_listener(var))
|
||||
```
|
||||
```cpp
|
||||
#ifdef MY_COMPONENT_LISTENER_COUNT
|
||||
void register_listener(MyComponentListener *listener);
|
||||
#endif
|
||||
protected:
|
||||
#ifdef MY_COMPONENT_LISTENER_COUNT
|
||||
StaticVector<MyComponentListener *, MY_COMPONENT_LISTENER_COUNT> listeners_;
|
||||
#endif
|
||||
```
|
||||
Request slots from `to_code`, not from a job that runs after `CoroPriority.FINAL` - a late
|
||||
request raises rather than silently undercounting.
|
||||
|
||||
3. **Runtime-known sizes:** Use `FixedVector` from `esphome/core/helpers.h` when the size is only known at runtime initialization.
|
||||
```cpp
|
||||
// Bad - generates STL realloc code (_M_realloc_insert)
|
||||
@@ -677,25 +599,9 @@ file does, and it is the authority when they disagree. The most useful starting
|
||||
```
|
||||
Linear search on small datasets (1-16 elements) is often faster than hashing/tree overhead, but this depends on lookup frequency and access patterns. For frequent lookups in hot code paths, the O(1) vs O(n) complexity difference may still matter even for small datasets. `std::vector` with simple structs is usually fine—it's the heavy containers (`map`, `set`, `unordered_map`) that should be avoided for small datasets unless profiling shows otherwise.
|
||||
|
||||
5. **Strings set once from configuration:** Use `StringRef` (`esphome/core/string_ref.h`) rather than
|
||||
`std::string`. Code generation passes a string literal that lives in flash for the life of the
|
||||
program, so storing a `std::string` copies it onto the heap for nothing. `StringRef` is a
|
||||
non-owning pointer plus length; it does not copy, and it must only ever refer to storage that
|
||||
outlives it (a string literal, or a buffer owned elsewhere).
|
||||
```cpp
|
||||
// Bad - heap copy of a literal that is already in flash
|
||||
void set_keys(std::string keys) { this->keys_ = std::move(keys); }
|
||||
std::string keys_;
|
||||
```
|
||||
```cpp
|
||||
// Good - no allocation
|
||||
void set_keys(const char *keys) { this->keys_ = StringRef(keys); }
|
||||
StringRef keys_;
|
||||
```
|
||||
5. **Avoid `std::deque`:** It allocates in 512-byte blocks regardless of element size, guaranteeing at least 512 bytes of RAM usage immediately. This is a major source of crashes on memory-constrained devices.
|
||||
|
||||
6. **Avoid `std::deque`:** It allocates in 512-byte blocks regardless of element size, guaranteeing at least 512 bytes of RAM usage immediately. This is a major source of crashes on memory-constrained devices.
|
||||
|
||||
7. **Detection:** Look for these patterns in compiler output:
|
||||
6. **Detection:** Look for these patterns in compiler output:
|
||||
- Large code sections with STL symbols (vector, map, set)
|
||||
- `alloc`, `realloc`, `dealloc` in symbol names
|
||||
- `_M_realloc_insert`, `_M_default_append` (vector reallocation)
|
||||
|
||||
@@ -16,7 +16,7 @@ name and promote with an atomic rename.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from collections.abc import Iterator
|
||||
from collections.abc import Iterable, Iterator
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
from contextlib import contextmanager, suppress
|
||||
import hashlib
|
||||
@@ -55,8 +55,8 @@ def _preserved_sys_path() -> Iterator[None]:
|
||||
sys.path[:] = saved
|
||||
|
||||
|
||||
# Concurrent registry resolutions / HEAD probes (each is network-bound)
|
||||
_RESOLVE_WORKERS = 8
|
||||
# Cap for network-bound work: resolutions, HEAD probes, clone floor
|
||||
_NETWORK_WORKERS = 8
|
||||
|
||||
# A hung child must not block the build; downloads resume on the next run
|
||||
_PREFETCH_TIMEOUT = 20 * 60
|
||||
@@ -341,7 +341,7 @@ def _registry_jobs(
|
||||
if not pending:
|
||||
return [], 0, []
|
||||
# Serial resolutions (registry GET + mirror HEAD each) dominate
|
||||
with ThreadPoolExecutor(max_workers=min(_RESOLVE_WORKERS, len(pending))) as ex:
|
||||
with ThreadPoolExecutor(max_workers=min(_NETWORK_WORKERS, len(pending))) as ex:
|
||||
results = list(ex.map(_resolve, pending))
|
||||
jobs: list[tuple[str, int, Any]] = []
|
||||
installable: list[tuple[str, Any]] = []
|
||||
@@ -374,14 +374,47 @@ def _registry_jobs(
|
||||
return jobs, failed, installable
|
||||
|
||||
|
||||
# The schemes pio's VCSClientFactory dispatches on (Git/Hg/SvnClient)
|
||||
_VCS_URI_PREFIXES = ("git+", "hg+", "svn+", "git://", "hg://", "svn://")
|
||||
|
||||
|
||||
def _is_vcs_spec_uri(url: str | None) -> bool:
|
||||
"""Whether pio's ``install_from_uri`` would clone this URI (PackageSpec
|
||||
normalizes git URLs to ``git+``). The .git check runs first so an
|
||||
un-normalized repo URL fails as a clone, not as an archive download."""
|
||||
if not url or url.startswith(("file://", "symlink://")):
|
||||
return False
|
||||
if url.split("#", 1)[0].endswith(".git"):
|
||||
return True
|
||||
if url.startswith(("http://", "https://")):
|
||||
return False
|
||||
return url.startswith(_VCS_URI_PREFIXES)
|
||||
|
||||
|
||||
# (name, spec) from wave 1, (name, spec, compatibility) from dep waves
|
||||
_Entry = tuple[str, Any] | tuple[str, Any, Any]
|
||||
|
||||
|
||||
def _entry_is_vcs(entry: _Entry) -> bool:
|
||||
"""Whether this pre-install entry is cloned rather than unpacked."""
|
||||
return _is_vcs_spec_uri(entry[1].uri)
|
||||
|
||||
|
||||
def _clones_first(entries: Iterable[_Entry]) -> list[_Entry]:
|
||||
"""Clones first: they wait on the network, so they must not queue
|
||||
behind CPU-bound archive extractions in the pre-install pool."""
|
||||
return sorted(entries, key=lambda entry: not _entry_is_vcs(entry))
|
||||
|
||||
|
||||
def _uri_jobs(
|
||||
manager: Any, specs: list[Any], seen: set[str]
|
||||
) -> tuple[list[tuple[str, int, Any]], int, list[tuple[str, Any]]]:
|
||||
"""Jobs for direct-URL specs; a HEAD sizes each for the combined bar.
|
||||
|
||||
Also returns how many HEAD probes errored (an absent length is not an
|
||||
error) and the ``(name, spec)`` pairs whose archives will be
|
||||
installable.
|
||||
error) and the ``(name, spec)`` pairs to pre-install: downloaded
|
||||
archives, plus VCS specs, which have no archive -- the pre-install
|
||||
itself clones them, in parallel instead of one at a time in pio run.
|
||||
"""
|
||||
from esphome.net_retry import fetch_with_retry, http_request
|
||||
|
||||
@@ -389,13 +422,25 @@ def _uri_jobs(
|
||||
installable: list[tuple[str, Any]] = []
|
||||
for spec in specs:
|
||||
url = spec.uri
|
||||
if not url or not url.startswith(("http://", "https://")):
|
||||
continue # git+/file specs are cloned/copied, not downloaded
|
||||
if url.split("#", 1)[0].endswith(".git"):
|
||||
continue # bare-URL VCS spec; PlatformIO clones it
|
||||
if not url:
|
||||
continue
|
||||
is_vcs = _is_vcs_spec_uri(url)
|
||||
if not is_vcs and not url.startswith(("http://", "https://")):
|
||||
if not url.startswith(("file://", "symlink://")):
|
||||
_LOGGER.debug(
|
||||
"Unrecognized package URI, leaving it to pio run: %s", url
|
||||
)
|
||||
continue # file/symlink specs are copied in place by pio run
|
||||
if manager.get_package(spec):
|
||||
continue
|
||||
name = spec.name or url.rsplit("/", 1)[-1]
|
||||
if is_vcs:
|
||||
# The pre-install clones it, gated like the cached-archive
|
||||
# branch below: only a custom name is the destination dir.
|
||||
# Platform tool specs always parse as custom-named
|
||||
if spec.has_custom_name():
|
||||
installable.append((name, spec))
|
||||
continue
|
||||
# PlatformIO downloads URL specs with no checksum
|
||||
dl_path = Path(manager.compute_download_path(url, ""))
|
||||
if dl_path.is_file():
|
||||
@@ -409,7 +454,7 @@ def _uri_jobs(
|
||||
if str(dl_path) in seen:
|
||||
continue # another spec already claimed this .part
|
||||
seen.add(str(dl_path))
|
||||
candidates.append((spec.name, url, dl_path, spec))
|
||||
candidates.append((name, url, dl_path, spec))
|
||||
|
||||
errors: list[str] = []
|
||||
|
||||
@@ -435,7 +480,7 @@ def _uri_jobs(
|
||||
|
||||
if not candidates:
|
||||
return [], 0, installable
|
||||
with ThreadPoolExecutor(max_workers=min(_RESOLVE_WORKERS, len(candidates))) as ex:
|
||||
with ThreadPoolExecutor(max_workers=min(_NETWORK_WORKERS, len(candidates))) as ex:
|
||||
sizes = list(ex.map(_head_size, [url for _, url, _, _ in candidates]))
|
||||
jobs: list[tuple[str, int, Any]] = []
|
||||
failed = 0
|
||||
@@ -583,10 +628,6 @@ def _uri_fetch_job(manager: Any, url: str, dl_path: Path, size: int) -> Any:
|
||||
return run
|
||||
|
||||
|
||||
# (name, spec) from wave 1, (name, spec, compatibility) from dep waves
|
||||
_Entry = tuple[str, Any] | tuple[str, Any, Any]
|
||||
|
||||
|
||||
def _dependency_entries(
|
||||
manager: Any, entries: list[_Entry], seen_names: set[str]
|
||||
) -> list[_Entry]:
|
||||
@@ -701,7 +742,14 @@ def _preinstall(
|
||||
would hang, not fail). Waves skip dependencies; the installed
|
||||
manifests feed the next wave. Any failure falls back to pio run.
|
||||
"""
|
||||
workers = min(get_usable_cpu_count(), len(entries))
|
||||
entries = _clones_first(entries)
|
||||
clones = sum(1 for entry in entries if _entry_is_vcs(entry))
|
||||
# Network-bound clones run wide even on small-core runners; capped
|
||||
# since each worker builds a sibling manager and may run a
|
||||
# postinstall, and a mixed wave's extractions inherit the width
|
||||
workers = min(
|
||||
max(get_usable_cpu_count(), min(clones, _NETWORK_WORKERS)), len(entries)
|
||||
)
|
||||
# One manager per worker (_install mutates instance state); built
|
||||
# serially because construction rewires the shared manager logger
|
||||
managers: SimpleQueue = SimpleQueue()
|
||||
@@ -733,7 +781,7 @@ def _preinstall(
|
||||
raise
|
||||
|
||||
_LOGGER.info(
|
||||
"Installing %d PlatformIO package(s) with %d extraction worker(s): %s",
|
||||
"Installing %d PlatformIO package(s) with %d worker(s): %s",
|
||||
len(entries),
|
||||
workers,
|
||||
", ".join(name for name, *_ in entries),
|
||||
|
||||
+6
-140
@@ -3,7 +3,6 @@
|
||||
import argparse
|
||||
import codecs
|
||||
import collections
|
||||
from collections.abc import Iterator
|
||||
import fnmatch
|
||||
import functools
|
||||
import os.path
|
||||
@@ -1121,56 +1120,7 @@ def lint_no_std_bind(fname, match):
|
||||
)
|
||||
|
||||
|
||||
LOG_CALL_START_RE = re.compile(r"ESP_LOG\w+\s*\(")
|
||||
# Comments, raw/plain string literals and single char literals are consumed whole so ; ( ) ? :
|
||||
# inside them are never seen. A char literal is exactly one (escaped) char so a digit separator
|
||||
# like 1'000'000 cannot open one.
|
||||
CPP_COMMENT_RE = r"//[^\n]*|/\*.*?\*/"
|
||||
CPP_SKIP_RE = (
|
||||
CPP_COMMENT_RE
|
||||
+ r'|R"(?P<raw_delim>[^(\s]*)\(.*?\)(?P=raw_delim)"|"(?:[^"\\]|\\.)*"|\'(?:[^\'\\\n]|\\.)\''
|
||||
)
|
||||
LOG_CALL_TOKEN_RE = re.compile(CPP_SKIP_RE + r"|[()]", re.DOTALL)
|
||||
# The last alternative matches a ? or : followed (after spaces or comments) by an opening quote,
|
||||
# i.e. a string literal used as a ternary branch.
|
||||
LOG_TERNARY_LITERAL_RE = re.compile(
|
||||
CPP_SKIP_RE + r"|[?:](?:\s|" + CPP_COMMENT_RE + r')*(?=")', re.DOTALL
|
||||
)
|
||||
# A bare NOLINT; a clang-tidy NOLINT(check-name) is aimed at a different tool.
|
||||
NOLINT_RE = re.compile(r"\bNOLINT\b(?!\()")
|
||||
|
||||
|
||||
def _line_col(content: str, pos: int) -> tuple[int, int]:
|
||||
"""1-based line and column of an offset in content."""
|
||||
return content.count("\n", 0, pos) + 1, pos - content.rfind("\n", 0, pos)
|
||||
|
||||
|
||||
def _iter_log_calls(content: str) -> Iterator[tuple[int, str | None]]:
|
||||
"""Yield (start, text) for every ESP_LOG*(...) call, text running to the matching close paren.
|
||||
text is None when no matching paren exists so callers can report the call instead of skipping it."""
|
||||
for head in LOG_CALL_START_RE.finditer(content):
|
||||
depth = 1
|
||||
for tok in LOG_CALL_TOKEN_RE.finditer(content, head.end()):
|
||||
if tok.group(0) == "(":
|
||||
depth += 1
|
||||
elif tok.group(0) == ")":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
yield head.start(), content[head.start() : tok.end()]
|
||||
break
|
||||
else:
|
||||
yield head.start(), None
|
||||
|
||||
|
||||
def _unbalanced_log_call_error(content: str, pos: int) -> tuple[int, int, str]:
|
||||
lineno, col = _line_col(content, pos)
|
||||
return (
|
||||
lineno,
|
||||
col,
|
||||
"ESP_LOG call has no matching closing parenthesis, so it cannot be checked.",
|
||||
)
|
||||
|
||||
|
||||
LOG_MULTILINE_RE = re.compile(r"ESP_LOG\w+\s*\(.*?;", re.DOTALL)
|
||||
LOG_BAD_CONTINUATION_RE = re.compile(r'\\n(?:[^ \\"\r\n\t]|"\s*\n\s*"[^ \\])')
|
||||
LOG_PERCENT_S_CONTINUATION_RE = re.compile(r'\\n(?:%s|"\s*\n\s*"%s)')
|
||||
|
||||
@@ -1178,16 +1128,16 @@ LOG_PERCENT_S_CONTINUATION_RE = re.compile(r'\\n(?:%s|"\s*\n\s*"%s)')
|
||||
@lint_content_check(include=cpp_include)
|
||||
def lint_log_multiline_continuation(fname, content):
|
||||
errs = []
|
||||
for log_start, log_text in _iter_log_calls(content):
|
||||
if log_text is None:
|
||||
errs.append(_unbalanced_log_call_error(content, log_start))
|
||||
continue
|
||||
for log_match in LOG_MULTILINE_RE.finditer(content):
|
||||
log_text = log_match.group(0)
|
||||
for bad_match in LOG_BAD_CONTINUATION_RE.finditer(log_text):
|
||||
# %s may expand to a whitespace prefix at runtime, skip those
|
||||
if LOG_PERCENT_S_CONTINUATION_RE.match(log_text, bad_match.start()):
|
||||
continue
|
||||
# Calculate line number from position in full content
|
||||
lineno, col = _line_col(content, log_start + bad_match.start())
|
||||
abs_pos = log_match.start() + bad_match.start()
|
||||
lineno = content.count("\n", 0, abs_pos) + 1
|
||||
col = abs_pos - content.rfind("\n", 0, abs_pos)
|
||||
errs.append(
|
||||
(
|
||||
lineno,
|
||||
@@ -1205,90 +1155,6 @@ def lint_log_multiline_continuation(fname, content):
|
||||
return errs
|
||||
|
||||
|
||||
def _find_ternary_literals(text: str) -> Iterator[tuple[int, str]]:
|
||||
"""Yield (offset, literal) for every string literal used as a ternary branch."""
|
||||
branch = False
|
||||
for m in LOG_TERNARY_LITERAL_RE.finditer(text):
|
||||
tok = m.group(0)
|
||||
# An empty literal is merged with every other string's terminator, so it costs no RAM,
|
||||
# while a PSTR("") would add its own flash array; leave it alone.
|
||||
if branch and tok[0] == '"' and tok != '""':
|
||||
yield m.start(), tok
|
||||
branch = tok[0] in "?:"
|
||||
|
||||
|
||||
# LOG_STR_LITERAL is a no op everywhere except ESP8266, so code that never builds there is skipped
|
||||
# to avoid churn: platform specific sources and components for ESP32, LibreTiny, RP2 and Zephyr only.
|
||||
# A component belongs here only if it has no tests/components/<name>/test.esp8266-ard.yaml.
|
||||
LOG_LITERAL_LINT_EXCLUDE = [
|
||||
"*_esp32.cpp",
|
||||
"*_esp32_*.cpp",
|
||||
"*_esp_idf.cpp",
|
||||
"*_rmt.cpp",
|
||||
"*_zephyr.cpp",
|
||||
"*_bk72xx.cpp",
|
||||
"*_libretiny.cpp",
|
||||
"*_pico_w.cpp",
|
||||
"*_host.cpp",
|
||||
"esphome/components/esp32*/*",
|
||||
"esphome/components/bk72xx*/*",
|
||||
"esphome/components/ln882h*/*",
|
||||
"esphome/components/ln882x*/*",
|
||||
"esphome/components/rp2*/*",
|
||||
"esphome/components/zephyr*/*",
|
||||
"esphome/components/host/*",
|
||||
"esphome/components/libretiny*/*",
|
||||
"esphome/components/bluetooth_proxy/*",
|
||||
"esphome/components/bluetooth_connection/*",
|
||||
"esphome/components/ble_client/*",
|
||||
"esphome/components/bedjet/*",
|
||||
"esphome/components/anova/*",
|
||||
"esphome/components/xiaomi_ble/*",
|
||||
"esphome/components/bthome_mithermometer/*",
|
||||
"esphome/components/usb_host/*",
|
||||
"esphome/components/zigbee/*",
|
||||
"esphome/components/lvgl/*",
|
||||
# Test fixtures and host only unit tests - not production embedded code
|
||||
"tests/integration/fixtures/*",
|
||||
"tests/components/*",
|
||||
]
|
||||
|
||||
|
||||
@lint_content_check(include=cpp_include, exclude=LOG_LITERAL_LINT_EXCLUDE)
|
||||
def lint_log_no_bare_literal_ternary(
|
||||
fname: Path, content: str
|
||||
) -> list[tuple[int, int, str]]:
|
||||
errs = []
|
||||
for log_start, log_text in _iter_log_calls(content):
|
||||
if log_text is None:
|
||||
continue # reported by lint_log_multiline_continuation, which sees every file
|
||||
# A NOLINT anywhere on the lines the call spans silences every branch in it
|
||||
first_line = content.rfind("\n", 0, log_start) + 1
|
||||
last_line = content.find("\n", log_start + len(log_text))
|
||||
if NOLINT_RE.search(
|
||||
content[first_line : last_line if last_line != -1 else None]
|
||||
):
|
||||
continue
|
||||
for offset, literal in _find_ternary_literals(log_text):
|
||||
lineno, col = _line_col(content, log_start + offset)
|
||||
errs.append(
|
||||
(
|
||||
lineno,
|
||||
col,
|
||||
(
|
||||
"String literal used as a ternary branch in a log call. On ESP8266 the "
|
||||
"log macro moves the format string to flash, but bare literal arguments "
|
||||
"stay in RAM. Wrap each branch passed straight to the log call in "
|
||||
f"{highlight('LOG_STR_LITERAL(...)')}:\n"
|
||||
f" Before: {highlight(literal)}\n"
|
||||
f" After: {highlight(f'LOG_STR_LITERAL({literal})')}\n"
|
||||
f"(If strictly necessary, add `{highlight('// NOLINT')}` to the end of the line)"
|
||||
),
|
||||
)
|
||||
)
|
||||
return errs
|
||||
|
||||
|
||||
@lint_content_find_check(
|
||||
"ESP_LOG",
|
||||
include=["*.h", "*.tcc"],
|
||||
|
||||
@@ -4,16 +4,12 @@ The rule flags an if/else/for/while whose only body is an unbraced ESP_LOG*() ca
|
||||
empty statement -- and a -Wempty-body warning -- once the log level compiles the macro out). These
|
||||
tests pin the comment/string/raw-string masker, the accepted control-statement shapes, and the
|
||||
NOLINT escape hatch at both placements a contributor would try.
|
||||
|
||||
Also covers the ESP_LOG call scanner (_iter_log_calls) and the bare-literal-ternary lint.
|
||||
"""
|
||||
|
||||
import importlib.util
|
||||
from pathlib import Path
|
||||
import sys
|
||||
|
||||
import pytest
|
||||
|
||||
SCRIPT_DIR = (Path(__file__).parent / ".." / ".." / "script").resolve()
|
||||
sys.path.insert(0, str(SCRIPT_DIR))
|
||||
_spec = importlib.util.spec_from_file_location("ci_custom", SCRIPT_DIR / "ci-custom.py")
|
||||
@@ -149,125 +145,3 @@ def test_nolint_at_end_of_log_line_suppresses() -> None:
|
||||
|
||||
def test_nolint_on_control_line_suppresses() -> None:
|
||||
assert not _lint("if (x) // NOLINT\n ESP_LOGD(t);\n")
|
||||
|
||||
|
||||
# --- ESP_LOG call scanner and bare-literal-ternary lint ---
|
||||
|
||||
|
||||
def _calls(content: str) -> list[str | None]:
|
||||
return [text for _, text in ci_custom._iter_log_calls(content)]
|
||||
|
||||
|
||||
def _ternary_errors(content: str) -> list[tuple[int, int]]:
|
||||
errs = ci_custom.lint_log_no_bare_literal_ternary(Path("x.cpp"), content)
|
||||
return [(line, col) for line, col, _ in errs]
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"content",
|
||||
[
|
||||
'ESP_LOGD(TAG, "a ) b ( c; d")',
|
||||
'ESP_LOGD(TAG, "quote \\" inside")',
|
||||
"ESP_LOGD(TAG, \"%s\", format_hex_pretty(x, '-', false).c_str())",
|
||||
"ESP_LOGD(TAG, \"%c%c\", '(', ')')",
|
||||
"ESP_LOGD(TAG, \"%d\", 1'000'000)",
|
||||
'ESP_LOGD(TAG, // it\'s a comment with ) and (\n "x")',
|
||||
'ESP_LOGD(TAG, /* :) */ "x")',
|
||||
'ESP_LOGD(TAG, "%s", R"(say "hi" :) )")',
|
||||
'ESP_LOGD(TAG, "%s", R"x(a)"b)x")',
|
||||
],
|
||||
)
|
||||
def test_iter_log_calls_spans_whole_call(content: str) -> None:
|
||||
calls = _calls(content + ";\nint other = (1);")
|
||||
assert calls == [content]
|
||||
|
||||
|
||||
def test_iter_log_calls_reports_unbalanced_call_once() -> None:
|
||||
content = 'ESP_LOGD(TAG, "x";\nvoid f();'
|
||||
assert _calls(content) == [None]
|
||||
errs = ci_custom.lint_log_multiline_continuation(Path("x.cpp"), content)
|
||||
assert len(errs) == 1
|
||||
assert errs[0][:2] == (1, 1)
|
||||
assert "no matching closing parenthesis" in errs[0][2]
|
||||
assert _ternary_errors(content) == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("content", "expected"),
|
||||
[
|
||||
# A ; inside the format string no longer cuts the call short
|
||||
('ESP_LOGD(TAG, "a; b\\nc %s", x);', [(1, 20)]),
|
||||
# A \n%s continuation is exempt since %s may expand to leading whitespace
|
||||
('ESP_LOGD(TAG, "a\\n%s", x);', []),
|
||||
('ESP_LOGD(TAG, "a\\n b");', []),
|
||||
],
|
||||
)
|
||||
def test_multiline_continuation_detection(
|
||||
content: str, expected: list[tuple[int, int]]
|
||||
) -> None:
|
||||
errs = ci_custom.lint_log_multiline_continuation(Path("x.cpp"), content)
|
||||
assert [(line, col) for line, col, _ in errs] == expected
|
||||
|
||||
|
||||
def test_exclusion_list_only_names_components_without_esp8266_tests() -> None:
|
||||
root = Path(__file__).parent / ".." / ".."
|
||||
for pattern in ci_custom.LOG_LITERAL_LINT_EXCLUDE:
|
||||
if not pattern.startswith("esphome/components/"):
|
||||
continue
|
||||
prefix = pattern.removeprefix("esphome/components/").split("/")[0]
|
||||
comps = list((root / "esphome" / "components").glob(prefix))
|
||||
assert comps, f"{pattern!r} matches no component"
|
||||
for comp in comps:
|
||||
test = root / "tests" / "components" / comp.name / "test.esp8266-ard.yaml"
|
||||
assert not test.exists(), (
|
||||
f"{comp.name} builds for ESP8266, drop {pattern!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_unbalanced_calls_are_reported_by_a_check_that_sees_every_file() -> None:
|
||||
# lint_log_no_bare_literal_ternary skips unbalanced calls and relies on this
|
||||
checks = {c["func"].__name__: c for c in ci_custom.LINT_CONTENT_CHECKS}
|
||||
continuation = checks["lint_log_multiline_continuation"]
|
||||
ternary = checks["lint_log_no_bare_literal_ternary"]
|
||||
assert continuation["exclude"] == []
|
||||
assert continuation["include"] == ternary["include"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("content", "expected"),
|
||||
[
|
||||
('ESP_LOGD(TAG, "%s", x ? "on" : "off");', [(1, 25), (1, 32)]),
|
||||
(
|
||||
'ESP_LOGD(TAG, "%s", x ? LOG_STR_LITERAL("on") : LOG_STR_LITERAL("off"));',
|
||||
[],
|
||||
),
|
||||
('ESP_LOGD(TAG, "%s", x ? LOG_STR_LITERAL("on") : "off");', [(1, 49)]),
|
||||
('ESP_LOGD(TAG, "%s", x ? "on" : "");', [(1, 25)]),
|
||||
(
|
||||
'ESP_LOGD(TAG, "%s",\n x ? "yes"\n : "no");',
|
||||
[(2, 14), (3, 14)],
|
||||
),
|
||||
("ESP_LOGD(TAG, \"%c\", x ? '1' : '0');", []),
|
||||
('ESP_LOGD(TAG, "a ? b : c %s", x ? "on" : "off");', [(1, 35), (1, 42)]),
|
||||
('ESP_LOGD(TAG, "x:" "y %s", p);', []),
|
||||
('ESP_LOGD(TAG, "%s", x ? "on" : "off"); // NOLINT', []),
|
||||
('ESP_LOGD(TAG, "%s",\n x ? "yes"\n : "no"); // NOLINT', []),
|
||||
('ESP_LOGD(TAG, "%s", x ? /* c */ "on" : "off");', [(1, 33), (1, 40)]),
|
||||
(
|
||||
'ESP_LOGD(TAG, "%s",\n x ? "on" // NOLINT(some-clang-check)\n : "off");',
|
||||
[(2, 14), (3, 14)],
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_ternary_literal_detection(
|
||||
content: str, expected: list[tuple[int, int]]
|
||||
) -> None:
|
||||
assert _ternary_errors(content) == expected
|
||||
|
||||
|
||||
def test_ternary_error_message_names_the_literal() -> None:
|
||||
errs = ci_custom.lint_log_no_bare_literal_ternary(
|
||||
Path("x.cpp"), 'ESP_LOGD(TAG, "%s", x ? "enabled" : LOG_STR_LITERAL("off"));'
|
||||
)
|
||||
assert len(errs) == 1
|
||||
assert 'LOG_STR_LITERAL("enabled")' in errs[0][2]
|
||||
|
||||
@@ -588,7 +588,8 @@ def test_registry_jobs_one_bad_spec_keeps_the_rest(tmp_path: Path) -> None:
|
||||
|
||||
|
||||
def test_uri_jobs_head_sizes_the_bar(tmp_path: Path) -> None:
|
||||
"""HEAD sizes direct-URL specs; git and unreachable URLs are skipped."""
|
||||
"""HEAD sizes direct-URL specs; VCS specs skip the download but are
|
||||
still installable (the pre-install clones them in parallel)."""
|
||||
m = _fake_manager(tmp_path)
|
||||
resp = MagicMock()
|
||||
resp.headers = {"content-length": "2222"}
|
||||
@@ -597,15 +598,14 @@ def test_uri_jobs_head_sizes_the_bar(tmp_path: Path) -> None:
|
||||
m,
|
||||
[
|
||||
_FakeSpec(uri="https://x/big.zip", name="big", custom_name=True),
|
||||
_FakeSpec(uri="git+https://x/repo.git", name="repo"),
|
||||
_FakeSpec(uri="https://x/repo.git#v1", name="barevcs"),
|
||||
_FakeSpec(uri="git+https://x/repo.git", name="repo", custom_name=True),
|
||||
_FakeSpec(name="registry"),
|
||||
],
|
||||
set(),
|
||||
)
|
||||
assert failed == 0
|
||||
assert [(n, s) for n, s, _ in jobs] == [("big", 2222)]
|
||||
assert [n for n, _ in installable] == ["big"]
|
||||
assert [n for n, _ in installable] == ["repo", "big"]
|
||||
# a successful HEAD with no Content-Length is a clean skip
|
||||
resp.headers = {}
|
||||
with patch("esphome.net_retry.http_request", return_value=resp):
|
||||
@@ -614,6 +614,67 @@ def test_uri_jobs_head_sizes_the_bar(tmp_path: Path) -> None:
|
||||
) == ([], 0, [])
|
||||
|
||||
|
||||
def test_uri_jobs_vcs_specs_installable_without_probe(tmp_path: Path) -> None:
|
||||
"""VCS specs never probe the network; custom-named uninstalled ones
|
||||
pre-install, everything else is left to pio run."""
|
||||
m = _fake_manager(tmp_path)
|
||||
with patch("esphome.net_retry.http_request") as mock_head:
|
||||
jobs, failed, installable = pf._uri_jobs(
|
||||
m,
|
||||
[
|
||||
_FakeSpec(
|
||||
uri="git+https://x/tool.git#1.0", name="tool", custom_name=True
|
||||
),
|
||||
_FakeSpec(uri="hg+https://x/old", name="mercurial", custom_name=True),
|
||||
_FakeSpec(uri="git+https://x/derived.git", name="derived"),
|
||||
# An un-normalized repo URL classifies as VCS (never as a
|
||||
# downloadable archive), then drops here as derived-name
|
||||
_FakeSpec(uri="https://x/unnorm.git", name="unnorm"),
|
||||
_FakeSpec(uri="file:///local/dir", name="local"),
|
||||
# A local .git path is copied in place, never cloned
|
||||
_FakeSpec(uri="file:///local/repo.git", name="localgit"),
|
||||
_FakeSpec(uri="symlink:///local/dir", name="link"),
|
||||
],
|
||||
set(),
|
||||
)
|
||||
mock_head.assert_not_called()
|
||||
assert (jobs, failed) == ([], 0)
|
||||
assert [n for n, _ in installable] == ["tool", "mercurial"]
|
||||
# Positive classification: an unknown scheme is left to pio run
|
||||
with patch("esphome.net_retry.http_request") as mock_head:
|
||||
assert pf._uri_jobs(
|
||||
m, [_FakeSpec(uri="weird://x/pkg", name="weird")], set()
|
||||
) == ([], 0, [])
|
||||
mock_head.assert_not_called()
|
||||
|
||||
m.get_package.return_value = object() # already installed: warm and silent
|
||||
with patch("esphome.net_retry.http_request"):
|
||||
assert pf._uri_jobs(
|
||||
m,
|
||||
[
|
||||
_FakeSpec(
|
||||
uri="git+https://x/tool.git#1.0", name="tool", custom_name=True
|
||||
)
|
||||
],
|
||||
set(),
|
||||
) == ([], 0, [])
|
||||
|
||||
|
||||
def test_clones_first_orders_vcs_before_archives() -> None:
|
||||
"""The pre-install pool receives clones first: they wait on the
|
||||
network and must not queue behind CPU-bound archive extractions."""
|
||||
archive = ("zip", _FakeSpec(uri="https://x/a.zip", name="zip"))
|
||||
registry = ("reg", _FakeSpec(uri=None, name="reg"))
|
||||
clone = ("repo", _FakeSpec(uri="git+https://x/repo.git", name="repo"))
|
||||
ordered = pf._clones_first([archive, registry, clone])
|
||||
assert ordered[0] == clone
|
||||
# Stable partition: non-clone relative order is preserved
|
||||
assert ordered[1:] == [archive, registry]
|
||||
assert pf._entry_is_vcs(clone)
|
||||
assert not pf._entry_is_vcs(archive)
|
||||
assert not pf._entry_is_vcs(registry)
|
||||
|
||||
|
||||
def test_uri_jobs_head_failure_counts_as_unresolved(
|
||||
tmp_path: Path, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
@@ -1600,12 +1661,9 @@ def test_preinstall_dependency_wave_skips_seen_names(tmp_path: Path) -> None:
|
||||
assert installed == ["noise-c"]
|
||||
|
||||
|
||||
def test_preinstall_uses_distinct_managers_in_parallel(tmp_path: Path) -> None:
|
||||
"""Each worker thread gets its own pre-built manager and installs
|
||||
genuinely overlap (the barrier deadlocks a serial pool). The worker
|
||||
count is pinned so a 1-CPU host cannot serialize the pool."""
|
||||
barrier = threading.Barrier(2, timeout=5)
|
||||
used: set = set()
|
||||
def _wave_manager(tmp_path, on_install):
|
||||
"""A minimal pio-manager stand-in for _preinstall pool tests;
|
||||
``on_install(manager, spec)`` observes each _install call."""
|
||||
|
||||
class _WaveManager:
|
||||
package_dir = str(tmp_path)
|
||||
@@ -1636,20 +1694,59 @@ def test_preinstall_uses_distinct_managers_in_parallel(tmp_path: Path) -> None:
|
||||
return None
|
||||
|
||||
def _install(self, spec, skip_dependencies, compatibility=None) -> None:
|
||||
used.add(id(self))
|
||||
barrier.wait()
|
||||
on_install(self, spec)
|
||||
|
||||
seed = _WaveManager(str(tmp_path))
|
||||
with patch.object(pf, "get_usable_cpu_count", return_value=2):
|
||||
return _WaveManager
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("cpu_count", "entries"),
|
||||
[
|
||||
(2, [("a@1", _FakeSpec(name="a")), ("b@1", _FakeSpec(name="b"))]),
|
||||
# The clone floor: network-bound clones run wide on a 1-CPU host
|
||||
(
|
||||
1,
|
||||
[
|
||||
(f"r{i}", _FakeSpec(uri=f"git+https://x/r{i}.git", name=f"r{i}"))
|
||||
for i in range(4)
|
||||
],
|
||||
),
|
||||
],
|
||||
ids=("cpu-sized", "clone-floor"),
|
||||
)
|
||||
def test_preinstall_pool_width(tmp_path: Path, cpu_count: int, entries: list) -> None:
|
||||
"""The barrier deadlocks unless every entry gets its own manager
|
||||
and runs concurrently."""
|
||||
barrier = threading.Barrier(len(entries), timeout=5)
|
||||
used: set = set()
|
||||
|
||||
def on_install(mgr, spec) -> None:
|
||||
used.add(id(mgr))
|
||||
barrier.wait()
|
||||
|
||||
cls = _wave_manager(tmp_path, on_install)
|
||||
seed = cls(str(tmp_path))
|
||||
with patch.object(pf, "get_usable_cpu_count", return_value=cpu_count):
|
||||
pf._preinstall(seed, entries)
|
||||
assert len(used) == len(entries)
|
||||
assert id(seed) not in used
|
||||
|
||||
|
||||
def test_preinstall_orders_clones_before_extractions(tmp_path: Path) -> None:
|
||||
"""With one worker, the clone installs before the archive
|
||||
regardless of caller order."""
|
||||
order: list[str] = []
|
||||
cls = _wave_manager(tmp_path, lambda mgr, spec: order.append(spec.name))
|
||||
seed = cls(str(tmp_path))
|
||||
with patch.object(pf, "get_usable_cpu_count", return_value=1):
|
||||
pf._preinstall(
|
||||
seed,
|
||||
[
|
||||
("a@1", _FakeSpec(name="a")),
|
||||
("b@1", _FakeSpec(name="b")),
|
||||
("zip", _FakeSpec(uri="https://x/a.zip", name="zip")),
|
||||
("repo", _FakeSpec(uri="git+https://x/repo.git", name="repo")),
|
||||
],
|
||||
)
|
||||
assert len(used) == 2
|
||||
assert id(seed) not in used
|
||||
assert order == ["repo", "zip"]
|
||||
|
||||
|
||||
def test_sibling_manager_and_sigterm() -> None:
|
||||
@@ -1803,3 +1900,13 @@ def test_platformio_private_api_contract() -> None:
|
||||
derived = PackageSpec("https://x/y/archive/master.zip")
|
||||
assert derived.name and not derived.has_custom_name()
|
||||
assert PackageSpec("Foo=https://x/y/archive/master.zip").has_custom_name()
|
||||
# _is_vcs_spec_uri relies on bare .git URLs normalizing to git+, on
|
||||
# both parse paths (raw string, and requirements= for platform tools)
|
||||
assert PackageSpec("https://github.com/x/y.git#v1").uri.startswith("git+")
|
||||
platform_tool = PackageSpec(
|
||||
owner="o", name="tool-x", requirements="https://github.com/x/y.git"
|
||||
)
|
||||
assert platform_tool.uri.startswith("git+")
|
||||
# A URL requirement re-parses as name=url, marking the name custom;
|
||||
# this is what keeps platform tool clones in the parallel pre-install
|
||||
assert platform_tool.has_custom_name()
|
||||
|
||||
Reference in New Issue
Block a user