Bump to 0.27.1: what the review round found - #192
Merged
Conversation
Two commits since v0.27.0, both fixes. No new capability, so a patch. One live bug: g_coredump was read and written across three tasks with no lock -- the erase action clears it from the AsyncTCP task while bridgeInfo() copies its std::string and backtrace from loop(), rs485Task and AsyncTCP, roughly four times a second. That is a use-after-free waiting for two cores to line up, not a stale read. Two dormant traps closed rather than documented away: the Modbus driver family never took the bus lock while the dispatcher's comment claimed it waited two seconds for one, and command outcomes were keyed on a caller-chosen request id alone, so polling one device could return another's result. Dispatch was never affected by the second -- only the answer an operator reads. Five documentation claims corrected, including one that said enum mode setpoints cannot be expressed when the profile driver has implemented them since July. VERIFIED: 1023 native cases, all eleven gates, four board builds. The 1CH build's four warnings are all inside eModbus, none in src/ -- checked again rather than carried over from the last note.
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the firmware patch version to 0.27.1 and records the release-notes narrative in src/main.cpp alongside prior version notes, aligning the in-source version constants with the stated patch release.
Changes:
- Bump
HELIOGRAPH_VERSION_PATCHfrom0to1(0.27.1). - Add a detailed 0.27.1 release-notes comment describing the fixes found during review.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+506
to
+509
| // Worth recording HOW it survived: the declaration's own comment named the mutation -- "the one | ||
| // thing that CAN change it" -- and then argued for three lines that the cached copy is accurate. | ||
| // Thirty lines above sits g_configMutex, written for this exact hazard, with a comment describing | ||
| // this exact shape. The reasoning stopped one step short of its own conclusion. |
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.
Two commits since
v0.27.0, both fixes. No new capability, so a patch.One live bug
g_coredumpwas read and written across three tasks with no lock. The erase action clears it fromthe AsyncTCP task while
bridgeInfo()copies itsstd::stringand backtrace fromloop(),rs485Taskand AsyncTCP itself — roughly four times a second. That is a use-after-free waitingfor two cores to line up, not a stale read.
Two dormant traps, closed rather than documented away
waited two seconds for one. Harmless while only
rs485Tasktouches a Transport — which isexactly why it was worth closing.
return another's result. Dispatch was never affected; only the answer an operator reads.
Five documentation corrections
Including one that said enum mode setpoints cannot be expressed, when the profile driver has
implemented them since July.
Verification
1023 native cases, all eleven gates, four board builds. The 1CH build's four warnings are all
inside eModbus, none in
src/— checked again rather than carried over from the last note.