Repository navigation
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.
|
Important Review skippedToo many files! This PR contains 118 files, which is 18 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
If this change should show up in the release notes, add one with |
Contributor
Cpp-Linter Report
|
hhvrc
commented
Oct 24, 2026
Comment on lines
+252
to
+254
| } else if (ec == std::errc::result_out_of_range) { | ||
| retryAfter = UINT32_MAX; | ||
| } |
hhvrc
commented
Oct 24, 2026
Comment on lines
+112
to
+115
| HubInfoResponse out {}; | ||
|
|
||
| out.hubId = std::move(*hubId); | ||
| out.hubName = std::move(*hubName); |
Contributor
Author
There was a problem hiding this comment.
why sint this out = { .whatever = move(*thing) }
hhvrc
commented
Oct 24, 2026
| CONFIG_LOCK_READ(false); | ||
|
|
||
| out = _configData.rf; | ||
| CONFIG_LOCK_READ(std::nullopt); |
hhvrc
commented
Oct 24, 2026
| } | ||
| } | ||
| template<typename T> // T inherits from ConfigBase<T> | ||
| bool FromJsonStrParsed(T& val, JSON::JsonView json, std::string_view name, bool (*StringParser)(T&, std::string_view), T defaultVal) |
Contributor
Author
There was a problem hiding this comment.
why use FunctionRef everywhere else but not here?
hhvrc
commented
Oct 24, 2026
| constexpr uint32_t Crc32(const T& data) | ||
| { | ||
| const auto bytes = std::bit_cast<std::array<uint8_t, sizeof(T)>>(data); | ||
| return Crc32(bytes.data(), bytes.size()); |
Contributor
Author
There was a problem hiding this comment.
cant we pass in bytes directly?
hhvrc
commented
Oct 24, 2026
Contributor
Author
There was a problem hiding this comment.
this needs a deeper review
hhvrc
commented
Oct 24, 2026
Contributor
Author
There was a problem hiding this comment.
Why is move only function used here
This branch has not been deployed
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.