Repository navigation
refactor: adopt C++23 idioms across the firmware - #538
Merged
Merged
Conversation
gen_env_header.py ran git rev-parse in its working directory, which CMake sets to the build directory. A build directory outside the repository therefore had no commit and failed on an incomplete openshock_version.h.
Firmware has been built at ESP-IDF's default -Og since the move off PlatformIO. CI builds of the stable channel (master, final tags and pull requests into master) now select CONFIG_COMPILER_OPTIMIZATION_SIZE, which shrinks the image by about 10% (Core-V2 1,292,752 to 1,160,960 bytes) and frees about 7.6 KB of IRAM on ESP32. -O2 was measured too: it produced a larger image than -Og for no measurable gain on this I/O-bound firmware. Develop, beta and local builds stay at -Og so backtraces and debugger sessions keep every variable. OPENSHOCK_FW_OPTIMIZATION=size|debug overrides the choice; a local size build gets its own build directory because an existing sdkconfig wins over the defaults. Variant ids are unchanged.
- Return std::optional or std::expected instead of bool plus an out parameter: config getters, JSON accessors, enum, SemVer and split parsers, JSON API responses, GPIO conversion and the OTA helpers. - Convert::To* become constexpr wrappers over std::from_chars; the From* functions and DigitCounter are removed. HexUtils and Checksum take string_view and span. - SemVer::toString uses std::to_chars with checked results, which -O2 rejected in its earlier form. - Captive portal parameters report Missing and Invalid separately, so a password that fails to decode is no longer treated as absent. urlDecode takes a string_view and bounds-checks percent escapes. - Add util/FunctionRef.h for non-owning callbacks; owning callbacks use std::move_only_function. - Use std::to_underlying, using-aliases and consteval where they apply, and drop default cases from exhaustive switches so new enumerators warn.
Contributor
|
Important Review skippedToo many files! This PR contains 121 files, which is 21 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.
 |
Contributor
Change file check✅ OK — 2 valid change file(s) added in this PR. |
hhvrc
commented
Oct 24, 2026
hhvrc
commented
Oct 24, 2026
hhvrc
commented
Oct 24, 2026
hhvrc
commented
Oct 24, 2026
hhvrc
commented
Oct 24, 2026
hhvrc
commented
Oct 24, 2026
hhvrc
commented
Oct 24, 2026
Stable builds now also select CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT, which removes the file, line and expression text of every assert: about 54 KB per board (Core-V2 1,158,576 to 1,104,608 bytes). A failed assert still aborts with a backtrace and core dump. To decode those backtraces, each variant build uploads its ELF and the release job attaches it to the GitHub release as OpenShock_<variant>_<version>.elf. Stamping does not move code, so one ELF covers every board of a variant. Adds the change files for this branch.
- Parse Retry-After with Convert::ToUint32, defaulting to and clamping at 2 to 600 seconds; RateLimiter is unchanged - Initialise HubInfoResponse with designated initialisers - Pass the bit_cast array straight to the Crc32 and Sum8 span overloads - Take FromJsonStrParsed's parser as a FunctionRef - FunctionRef: delete assignment from a callable, which left the reference pointing at a destroyed temporary, and reject member pointers, which were referenced by the address of a temporary; both match std::function_ref - Apply clang-format 21
A restored CI install puts $IDF_PATH/tools on PATH, and build.py took the idf.py found there for a wrapper that sets up its own environment. It is the plain script: its `env python` shebang takes whichever interpreter leads PATH, and since setup-python runs after the ESP-IDF activation that is the runner's Python, which lacks esp_idf_monitor. Every cache-hit build failed with "No module named 'esp_idf_monitor'". Run that script with the venv interpreter like the non-wrapper path does.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.