fix: two HAL stub gaps blocking firmware builds against the simulator - #30
fix: two HAL stub gaps blocking firmware builds against the simulator#30sfoulad wants to merge 2 commits into
Conversation
CrossPoint firmware's HalDisplay::displayBuffer takes a third forceCleanBaseOnHalf argument controlling whether a HALF_REFRESH forces a base-clearing waveform pass. Firmware that calls the 3-arg form fails to compile against this simulator's 2-arg stub, so the simulator cannot build at all until the signature matches. The simulator has no e-ink half-refresh base to keep clean, so the flag is accepted and ignored -- same behavior as every other refresh-quality hint this stub already no-ops.
esp_http_client_method_t stops at HTTP_METHOD_PUT, so firmware HTTP client code that issues a DELETE request (e.g. a device sign-out endpoint) fails to compile against this stub. Real ESP-IDF's esp_http_client_method_t defines HTTP_METHOD_DELETE; add it here plus the matching methodName() case. esp_http_client_open() already performs the whole request in one call for methods with no body, which suits DELETE.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (3)src/Hal*.{cpp,h}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/**/*.{cpp,h}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/HalDisplay.cpp📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🔇 Additional comments (3)
📝 WalkthroughWalkthroughThe change extends ChangesDisplay buffer API
HTTP DELETE method
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What
Two build-blocking gaps between this simulator's HAL stubs and current CrossPoint firmware, found while running the simulator against sfoulad/midad-by-foulad, an Arabic-first CrossPoint Reader fork.
Both were previously carried as uncommitted local patches to the checked-out
.pio/libdeps/simulator/copy in that fork, re-applied by hand after everypio clean/ libdeps refetch, because there was no upstream fix to depend on. Landing them here removes that maintenance burden for us and for any other fork on a current firmware checkout.1.
HalDisplay::displayBufferarity (src/HalDisplay.h,src/HalDisplay.cpp)Current CrossPoint firmware's
HalDisplay::displayBuffertakes a third argument,forceCleanBaseOnHalf, controlling whether aHALF_REFRESHforces a base-clearing waveform pass before the requested refresh. This simulator's stub still only takes(mode, turnOffScreen), so any firmware built against the current API fails to compile against the simulator at all — this is a hard build blocker, not just a fidelity gap.Fix: add the third parameter to both the header and implementation and no-op it. The simulator has no e-ink half-refresh base to keep clean, so accepting-and-ignoring the flag is correct — consistent with how this stub already treats other refresh-quality hints.
2. Missing
HTTP_METHOD_DELETE(src/esp_http_client.h)The
esp_http_client_method_tstub stops atHTTP_METHOD_PUT. Real ESP-IDF'sesp_http_client_method_tdefinesHTTP_METHOD_DELETE(seecomponents/esp_http_client/include/esp_http_client.hupstream). Firmware HTTP client code that issues a DELETE request (e.g. a device sign-out endpoint) fails to compile against this stub.Fix: add
HTTP_METHOD_DELETEto the enum and the matchingcaseinmethodName().esp_http_client_open()already performs the whole request in one call for methods with no body, which suits DELETE naturally.Not included: the
O_WRONLYvsO_RDWRfidelity gapWe were also carrying a local patch for
HalStorage::openFileForWriteopeningO_WRONLYinstead ofO_RDWR(breaking read-back through an open write handle mid-build). While preparing this PR I found that's already fixed upstream in #27 (9b1bece) — no action needed there.Testing
Both changes are mechanical HAL-stub signature/enum additions with no behavioral branching beyond the no-op default. Verified:
g++ -fsyntax-only -std=c++2aagainst both modified headers in isolation (clean, no repo build toolchain available in this environment to do a full firmware+simulator build here).midad-by-fouladfork's local build for several weeks with no issues.displayBuffersignature or the oldesp_http_client_method_tenum size.