RTC: report failures from every setter, add isValid(), make timerSleep fail-closed - #368
Merged
Merged
Conversation
The RTC setters and IRQ controls could not tell a communication failure
from success: setDateTime, clearIRQ and disableIRQ returned void,
setAlarmIRQ returned an int that was really the "alarm enabled" flag, and
setTimerIRQ returned 0 both for a requested stop and for a failed write.
Contract
- RTC_Base virtuals and the RTC_Class facade return bool: true only when
the RTC acknowledged every write and is in the requested state.
- setTimerIRQ(msec, uint32_t* applied_msec) reports the programmed period
through the out-pointer (the shape of Power_Class::setChargeCurrent); the
out-pointer is untouched on false and optional. The uint32_t-returning
setTimerIRQ(msec) is gone (a deprecated overload would have needed a
using-declaration in every driver and a second removal round; 0.3.0 lists
the API under Breaking changes anyway). setAlarmIRQ(int seconds), which
programmed the timer and not the alarm, is removed; it is declared
"= delete" so that setAlarmIRQ(0) is a compile error instead of silently
binding to the tm* overload.
- rtc_time_t / rtc_date_t / rtc_datetime_t gain isValid(); the
value-returning getTime()/getDate()/getDateTime() return every field as
-1 on failure, the pointer getters leave their output untouched.
- Alarm fields: -1 is the only wildcard (minutes, hours, date, weekDay;
seconds, month and year are ignored). RTC_Base::validateAlarmFields is
applied by the facade and by every driver, canSetAlarm()/hasTimerIRQ()
let a driver veto a request before anything is written.
- setDateTime rejects out-of-range fields and a year the chip cannot store
(PCF8563 1900-2099, RX8130 and PowerHub 2000-2099); the tm overloads
range-check the int fields before narrowing them; the weekday is
computed only for weekDay == -1.
Drivers
- The alarm interrupt is enabled only after the complete alarm value was
accepted, a stale alarm flag is cleared before re-arming (also when the
alarm is cleared), and a failed request never leaves a partial or old
alarm armed. The initial disable is retried.
- Timer operations preserve a pending alarm flag and vice versa (PCF8563
Control_status_2 is write-0-to-clear, RX8130 flag masks). PCF8563 stops
the running timer before loading a new period and enables TIE last, reads
Control_status_2 with a status, and clamps 1-499 ms to the 1 s minimum
step. RX8130 disableIRQ also stops the timer (TE) with a read-back retry,
and requests of 250 ms and more are never put on the 4096 Hz clock, whose
122 us /INT pulse the M5PM1 relay cannot catch.
- On PaperMono / ToughC5 the M5PM1 relay's stale wake source and IRQ status
are cleared between stopping the old source and arming the new one; a
relay failure is part of the result and the request is rolled back.
- RTC_PowerHub is rewritten against the STM32 firmware register map: plain
binary registers, alarm = minute/hour/day + enable written in that order,
no periodic timer, no readable alarm flag (0xD4 is not a usable "applied"
flag on firmware 0xF3), true means the STM32 accepted the request. begin()
probes the firmware version, getVoltLow() no longer reads an unrelated
register, the year and weekday bytes are validated on read (the weekday
table {1,2,4,8,10,20,40} round-trips through the firmware's BCD
conversion), a weekday-only alarm and day 31 (firmware limit) are refused.
- RTC_Base has a virtual destructor (deleted through std::unique_ptr).
- examples/Basic/Rtc checks isValid() before indexing the weekday table and
reports a failed setDateTime().
Breaking changes: return types of setDateTime/setDate/setTime,
setAlarmIRQ(date, time), clearIRQ and disableIRQ changed (void/int to
bool); a cleared alarm now reports true instead of 0; setAlarmIRQ no longer
returns -1 without an RTC; a pointer to member setTimerIRQ is ambiguous;
RTC_Base subclasses must update their overrides. Rebuild everything that
links against the library.
…wake
timerSleep powered the device off even when the wake-up alarm had not been
armed, and returned void so the caller could not tell.
- The three overloads return bool: if the function returns, sleep was not
entered. Reasons: invalid arguments, an RTC without the needed feature
(PowerHub has no timer, use the date/time overloads), a failed cleanup of
the existing IRQ sources, a failed RTC setup, a failed wake-source
registration, the PC build.
- Everything is validated (including the driver's own limits through
canSetAlarm/hasTimerIRQ) before the existing IRQ sources are touched, so
a bad request changes nothing. A request whose alarm fields are all
wildcards is refused: the alarm API treats that as "clear", which must
not be mistaken for an armed wake-up.
- The old timer and alarm are disabled and cleared (both attempted, then
combined) before the new one is armed; the ESP timer wake-up is set to
the period the RTC actually applied so both sources share the deadline,
and an ESP timer left by an earlier light sleep is dropped before an
RTC-only sleep ("not registered" counts as clean). Without an RTC the
ESP timer wake-up is used as before and its registration must succeed.
- _powerOff / _timerSleep report a cancelled sleep: a failed EXT0 / GPIO /
EXT1 wake registration, a wake pin that cannot be released, a PowerHub
that does not accept the power-off request, a light sleep that cannot be
entered. On cancellation the display is woken up again, a GPIO wake-up
armed here is released (only the RTC pin, the GPIO wake source may carry
the application's pins), and the caller releases the RTC IRQ and the ESP
timer it armed. Without an RTC the RTC IRQ pin is not registered.
- Docs: timerSleep(seconds) owns the ESP timer wake source; the RTC alarm
wakes only boards where its IRQ is wired to a wake route (an external RTC
unit does not wake the board).
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.
Summary
The RTC setters and IRQ controls could not tell a communication failure from success:
setDateTime,clearIRQanddisableIRQreturnedvoid,setAlarmIRQreturned anintthat was really the "alarm enabled" flag, andsetTimerIRQreturned 0 both for a requested stop and for a failed write.Power_Class::timerSleepthen powered the device off even when the wake-up alarm had not been armed.This PR makes every RTC setter report whether the RTC is in the requested state, adds
isValid()to the RTC value types, and makestimerSleeprefuse to sleep unless a wake-up is actually armed.API
setDateTime/setDate/setTimevoidbool(every register write acknowledged; out-of-range field or unsupported year → false)setTimerIRQ(msec, uint32_t* applied_msec = nullptr)uint32_t(0 = stopped or failed)bool, the programmed period through the optional out-pointer (same shape assetChargeCurrent)setAlarmIRQ(date, time)int(alarm enabled?)bool(alarm in the requested state; a request with no alarm field clears the alarm and returns true)clearIRQ/disableIRQvoidboolgetTime()/getDate()/getDateTime()(value-returning)isValid()to checkrtc_time_t/rtc_date_t/rtc_datetime_tisValid()(all fields set and in range)RTC_Class::hasTimerIRQ()/canSetAlarm()Power::timerSleep(...)×3voidbool: returns only when sleep was not entered (false)Behaviour
timerSleepis fail-closed: the old IRQ sources must be disabled and cleared, the new one armed, and a wake route registered; otherwise the device stays awake and everything armed by the call is released again. Invalid arguments, an RTC without the needed feature (PowerHub has no timer) and the PC build return false before any RTC state is touched.setDateTimerejects a year the chip cannot store instead of wrapping it (PCF8563 1900-2099, RX8130 / PowerHub 2000-2099).RTC_PowerHub: rewritten against the STM32 firmware register map (plain binary registers, alarm = minute/hour/day + enable, no timer, no readable alarm flag).begin()probes the firmware,getVoltLow()no longer reads an unrelated register, weekday values are validated on read.Breaking changes
setDateTime/setDate/setTime,setAlarmIRQ(date, time),clearIRQ,disableIRQ(void/int → bool),timerSleep(void → bool). Calls that ignore the result still compile. A return type is not part of the mangled name: rebuild everything that links against the library.setAlarmIRQ(date, time): a request with no alarm field (a clear) used to return 0 and now returns true; code that used theintas "alarm enabled" must be revisited. Without an RTC it returns false instead of −1.setTimerIRQ(msec)returnsboolinstead of the programmed period.uint32_t n = setTimerIRQ(ms);still compiles but stores 0/1; read the period throughsetTimerIRQ(ms, &n).setAlarmIRQ(int seconds)(deprecated by comment since 0.2.x; it programmed the timer, not the alarm) is removed. UsesetTimerIRQ(seconds * 1000). The overload is declared= deleteso thatsetAlarmIRQ(0)is a compile error instead of silently binding to thetm*overload.RTC_Basesubclasses: the virtuals changed (setTimerIRQ(msec, applied*), bool returns, virtual destructor) and must be updated.Power::timerSleep(int)on PowerHub returns false (no RTC timer); use the date/time overloads.setTimerIRQ(1..499 ms)on PCF8563 now arms the 1 s minimum step instead of stopping the timer.getTime()/getDate()/getDateTime()(value-returning) now return all −1 on failure (year was 2000 before).Known limitations (unchanged behaviour, now documented)
Verification
Builds: ESP32 / S3 / C5 / P4 (Arduino) and native, no new warnings. Independent adversarial reviews (two lenses, thirteen rounds) until no implementation finding remained. CI run on the ainyan03 fork before this PR.