diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e15d87..102931a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -288,6 +288,23 @@ The project follows Semantic Versioning once the first firmware release is tagge ### Fixed +- the **GPS session trigger now starts a session**. Choosing it armed the device and showed + `WAITING FOR THE LINE`, and nothing could ever start the session: only a manual START and + IMU launch detection reached `begin_session`. The mechanism existed - `gate_automation` + starts on pit exit and stops on pit entry, host-tested, with `session_trigger.cpp` already + setting `pit_exit_auto_start_enabled` from the trigger - and the crossings were already + being read off the timing queue and discarded in an empty loop. The two halves had never + been introduced +- a session started by a gate crossing now runs for **the configured duration**. + `SessionController::start` takes its duration from the configuration already saved, and + that was only ever written inside the manual start path, so a crossing-started session + would have used `SessionConfiguration`'s own twenty-minute default - plausible, and wrong + for anyone who set something else. The configuration now follows the settings: written at + boot and whenever a duration is edited +- a session ended by a pit entry is **recorded as a pit entry**. The summary's completion + reason was hardcoded to `driver_stop`, which was harmless while a driver stop was the only + way a session could end + - recorded G peaks were bounded by what the radar can draw rather than by what the car did. The display clamps a reading to 2 g so the dot stays on the dial, and that clamp was applied before the peaks were taken - so a session summary recorded the limit as though diff --git a/firmware/main/screen_router.cpp b/firmware/main/screen_router.cpp index 9bd7137..90fe0f2 100644 --- a/firmware/main/screen_router.cpp +++ b/firmware/main/screen_router.cpp @@ -12,6 +12,7 @@ #include "track_timer/diagnostics/snapshot.hpp" #include "track_timer/storage/sd_card.hpp" #include "track_timer/storage/sd_track_store.hpp" +#include "track_timer/session/gate_automation.hpp" #include "track_timer/timing/embedded_runtime.hpp" #include "track_timer/timing/engine.hpp" #include "track_timer/timing/settings_adapter.hpp" @@ -513,6 +514,14 @@ class ScreenRouter { // Unconditional: if the save failed, settings_ still holds what the device will // use for the next session, and that is what the driver should be shown. refresh_ready(); + // The same reasoning as refresh_ready, for the controller rather than the screen: + // a duration edited in the menu has to be the one a pit-exit crossing starts a + // session with. Only while nothing is running, because save_configuration is a + // transition the controller refuses mid-session anyway and the running session + // keeps the duration it began with. + if (!session_was_active_) { + configure_session(static_cast(esp_timer_get_time() / 1000)); + } } // Advances the session clock and refreshes whichever running-session screen is up. @@ -1130,6 +1139,10 @@ class ScreenRouter { // Restore the track that was selected before the last reboot. restore_selected_track(); + // Configured at boot, not only when a session is started by hand. A pit-exit + // crossing starts the session through the controller directly, and the controller + // takes the duration from whatever configuration it already holds. + configure_session(static_cast(esp_timer_get_time() / 1000)); refresh_ready(); // Whatever is already on the card, so Review opens on the last session driven // rather than on nothing after a power cycle. @@ -1281,20 +1294,33 @@ class ScreenRouter { refresh_pending(); } - void begin_session() noexcept + // Puts the current durations into the controller and leaves it ready to start. + // + // Split out of begin_session because a gate crossing can now start a session without + // going through it: SessionController::start takes its duration from the configuration + // already saved, so a controller that has never been configured starts a session of + // zero length and lands straight in overtime. Configuration therefore follows the + // settings, not the act of starting. + void configure_session(const std::int64_t now_ms) noexcept { - armed_ = false; - const auto now_ms = static_cast(esp_timer_get_time() / 1000); session::SessionConfiguration configuration{}; configuration.session_duration_ms = static_cast(settings_.session_duration_seconds) * 1'000; configuration.rest_duration_ms = static_cast(settings_.rest_duration_seconds) * 1'000; - if (session_.save_configuration(configuration, now_ms) != + if (session_.save_configuration(configuration, now_ms) == session::TransitionResult::accepted) { - (void)session_.enter_configuration(now_ms); - (void)session_.save_configuration(configuration, now_ms); + return; } + (void)session_.enter_configuration(now_ms); + (void)session_.save_configuration(configuration, now_ms); + } + + void begin_session() noexcept + { + armed_ = false; + const auto now_ms = static_cast(esp_timer_get_time() / 1000); + configure_session(now_ms); const auto started = session_.start(now_ms); ESP_LOGI("track_timer", "session start: %d over %u s", static_cast(started), static_cast(settings_.session_duration_seconds)); @@ -1435,6 +1461,45 @@ class ScreenRouter { // queue that fills and then reports drops, and a drop there would be this code's // fault rather than the engine's or the receiver's - which is exactly the distinction // the loss counters exist to preserve. + // Hands one gate crossing to the session automation and makes the rest of the shell + // agree with whatever it decided. + // + // Every outcome is logged, including the refusals. A pit exit that did not start a + // session looks exactly like no pit exit at all from outside, and the reasons are + // different enough to matter: the setting is off, the session was already running, or + // the controller refused the transition. + void apply_gate_automation(const timing::GateCrossingDecision& decision) noexcept + { + const auto now_ms = static_cast(esp_timer_get_time() / 1000); + const auto outcome = + session::apply_gate_session_automation(decision, now_ms, settings_, session_); + if (outcome.result == session::GateAutomationResult::ignored_non_event || + outcome.result == session::GateAutomationResult::ignored_gate) { + // Start and finish crossings come through this queue too, several per lap. + // Logging them would bury the two that mean something. + return; + } + ESP_LOGI("track_timer", "gate automation: %s -> %s", + timing::timing_gate_name(decision.gate), + session::gate_automation_result_name(outcome.result)); + if (!outcome.transitioned) { + return; + } + // The session was started or stopped from underneath the shell, so the two things + // begin_session and advance_session_phase would have done have to happen here. + // service_session picks up the state change itself and loads the right screen. + armed_ = false; + if (outcome.result == session::GateAutomationResult::stopped) { + // stop_from_pit_entry goes straight to review, having already refreshed the + // session times, so the summary is captured after it rather than before as + // the driver-stop path does - and confirm_stop is not part of this route. + capture_summary(); + (void)session_.complete_review(now_ms); + } + describe_mode(); + refresh_pending(); + } + void drain_timing_output() noexcept { if (!timing_started_) { @@ -1449,12 +1514,22 @@ class ScreenRouter { static_cast(duration_ms / 1'000 % 60), static_cast(duration_ms % 1'000)); } - // Drained and discarded for now. The records are the raw material for the offline - // replay in #43 and belong in the log rather than on the screen, but the queue has - // to be emptied either way: one left to fill would report drops that mean only - // that nobody was reading it. The runtime counts what passes through. + // Each crossing is offered to the session automation, which starts on pit exit + // and stops on pit entry when the settings ask for it. This is what the GPS + // trigger has always meant and never did: the automation and the queue both + // existed, and the records were read off the queue and thrown away. + // + // Offered unconditionally rather than only while armed on GPS, because the two + // settings the automation consults are the authority on whether it should act - + // pit_exit_auto_start_enabled is derived from the trigger, and pit-entry + // auto-stop is its own setting that a driver can want with any trigger. Gating + // here as well would mean two places deciding the same thing. + // + // The records are also the raw material for the offline replay in #43, which is + // why they keep being drained to the floor afterwards rather than consumed. timing::GateCrossingDecision decision{}; while (timing::try_receive_gate_crossing_record(decision)) { + apply_gate_automation(decision); } log_timing_progress(); } @@ -2056,6 +2131,20 @@ class ScreenRouter { // What the car actually did, taken at the moment the session ends and before anything // resets. Peaks come from the meter, which has been scoped to the session since it // started, so they describe this session rather than everything since boot. + [[nodiscard]] static logger::SessionCompletionReason summary_completion_reason( + const session::CompletionReason reason) noexcept + { + switch (reason) { + case session::CompletionReason::none: + return logger::SessionCompletionReason::none; + case session::CompletionReason::driver_stop: + return logger::SessionCompletionReason::driver_stop; + case session::CompletionReason::pit_entry: + return logger::SessionCompletionReason::pit_entry; + } + return logger::SessionCompletionReason::none; + } + void capture_summary() noexcept { const auto snapshot = session_.snapshot(); @@ -2069,7 +2158,11 @@ class ScreenRouter { // end, which is why one is never greater than the other. summary.session_duration_ms = std::max(0, snapshot.session_elapsed_ms); summary.session_overrun_ms = std::max(0, snapshot.session_overrun_ms); - summary.completion_reason = logger::SessionCompletionReason::driver_stop; + // Taken from the controller rather than assumed. Hardcoding driver_stop was + // harmless while a driver stop was the only way a session could end; a pit entry + // can now end one, and a record that says the driver stopped when they did not is + // worse than no record of how it ended. + summary.completion_reason = summary_completion_reason(snapshot.completion_reason); summary.integrity = logger::SummaryIntegrity::complete; // Honest rather than flattering: there is no receiver, so every session so far is // recorded without the lap data a driver would expect. diff --git a/tests/cpp/test_gate_session_automation.cpp b/tests/cpp/test_gate_session_automation.cpp index d31022e..5116289 100644 --- a/tests/cpp/test_gate_session_automation.cpp +++ b/tests/cpp/test_gate_session_automation.cpp @@ -125,6 +125,58 @@ int main() assert(std::strlen(session::gate_automation_result_name(result)) > 0); } + // A pit exit starts the session through the controller directly, so it runs for + // whatever configuration the controller already holds - not for whatever the driver + // set. A controller nobody has configured starts a session at SessionConfiguration's + // own default, which is a plausible twenty minutes and therefore the kind of wrong + // that does not look wrong. The configuration has to be written from the settings + // before a crossing can arrive, which is why it is no longer done only inside the + // manual start path. + settings.pit_exit_auto_start_enabled = true; + session::SessionController unconfigured{}; + decision = session::apply_gate_session_automation( + event(timing::TimingGate::pit_exit), 0, settings, unconfigured); + assert(decision.result == session::GateAutomationResult::started); + assert(unconfigured.snapshot().session_remaining_ms == + session::SessionConfiguration{}.session_duration_ms); + + // Configured to something other than the default, so a session that honoured the + // default rather than the setting is distinguishable from one that honoured neither. + session::SessionController configured{}; + session::SessionConfiguration configuration{}; + configuration.session_duration_ms = 35 * 60 * 1'000; + configuration.rest_duration_ms = 15 * 60 * 1'000; + assert(configuration.session_duration_ms != + session::SessionConfiguration{}.session_duration_ms); + // save_configuration is only accepted from the configuring state, which is why + // configure_session in the router falls back to entering it first. + assert(configured.enter_configuration(0) == session::TransitionResult::accepted); + assert(configured.save_configuration(configuration, 0) == + session::TransitionResult::accepted); + decision = session::apply_gate_session_automation( + event(timing::TimingGate::pit_exit), 10, settings, configured); + assert(decision.result == session::GateAutomationResult::started); + assert(configured.snapshot().session_remaining_ms == configuration.session_duration_ms); + assert(configured.snapshot().state == session::SessionState::running); + + // A second crossing while that session runs must not restart it, however many times + // the car passes the pit exit. + const auto started_remaining = configured.snapshot().session_remaining_ms; + decision = session::apply_gate_session_automation( + event(timing::TimingGate::pit_exit), 20, settings, configured); + assert(decision.result == session::GateAutomationResult::ignored_state); + assert(!decision.transitioned); + assert(configured.snapshot().session_remaining_ms == started_remaining); + + // And a pit entry ends it with its own reason rather than looking like a driver stop, + // which is what the summary records. + settings.pit_entry_auto_stop_enabled = true; + decision = session::apply_gate_session_automation( + event(timing::TimingGate::pit_entry), 5'000, settings, configured); + assert(decision.result == session::GateAutomationResult::stopped); + assert(configured.snapshot().state == session::SessionState::review); + assert(configured.snapshot().completion_reason == session::CompletionReason::pit_entry); + std::cout << "Lap-gate settings and independent pit session automation passed\n"; return 0; }