Skip to content

Bug Fixes & Code Cleanup (1.3.0)#6

Open
SeanathanVT wants to merge 15 commits into
mainfrom
cleanup/claude-code-refactor
Open

Bug Fixes & Code Cleanup (1.3.0)#6
SeanathanVT wants to merge 15 commits into
mainfrom
cleanup/claude-code-refactor

Conversation

@SeanathanVT

@SeanathanVT SeanathanVT commented Jun 12, 2026

Copy link
Copy Markdown
Owner

Summary

  • Three pre-existing bugs fixed: session start time, CSV export race condition, and ~10% timer drift
  • Belt-control routes converted from GET to POST to prevent accidental triggers
  • Internal cleanups: merged duplicate functions, deduplicated template JS, removed dead constants and imports

Bug Fixes

  • session_history.json recorded the save time as both start_time and end_time. Records now correctly capture the real start time from when the session begins.
  • The history load used by /export_csv was not holding _history_lock, meaning a concurrent session save could produce a corrupt read.
  • Stats monitor timer ran ~10% fast: the ask_stats() poll (~100ms) plus a 1s sleep made each loop iteration ~1.1s, drifting ~6 minutes over an hour. Replaced with time.monotonic()-based elapsed time.

Changed

  • All state-mutating routes (/start, /pause, /resume, /end_session, speed controls) now require POST — previously accepting GET meant a browser prefetch or back-button press could send a belt command. Templates updated from <a href> links to <form method="post"> buttons.

Internal Cleanup

  • Merged _load_full_session_history() into _load_session_history(limit=None) — the two were identical minus a limit slice and a missing lock on the full variant.
  • Extracted showShutdownOverlay() into base.html; removed from three template copies.
  • Removed duplicate KMH_TO_MPH constant (same value as KM_TO_MI), unused sys import, and a stale logging comment.
  • Removed /manual_reconnect route alias; renamed startupinfopopen_kwargs in run.py.

Test Plan

  • Start a session, walk for a few minutes, end it — verify start_time and end_time in session_history.json are different and correct
  • End a session and immediately export CSV — verify no corrupt rows
  • Run a 5-minute session and verify the timer matches wall clock closely
  • End a session and immediately start a new one — verify the belt starts and the session runs normally
  • Pause and resume mid-session — verify the belt restarts and the timer continues from where it left off
  • Confirm all speed buttons, Pause, Resume, Start, and End Session work as expected (POST forms)
  • Confirm browser back button after a speed change does not re-send the command
  • Confirm Close / Ctrl+C shows the shutdown overlay on all three session pages
  • Confirm an in-progress session is saved to history on graceful shutdown

SeanathanVT and others added 15 commits June 12, 2026 15:47
_build_session_record() called datetime.now() for both start_time and
end_time, so every saved session had start_time == end_time. Added
_session_start_time global, set it when the session begins in
start_session(), and reset it to None in end_session().

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Every other history function holds _history_lock before touching
session_history.json, but _load_full_session_history (called by the
CSV export route) did not. A concurrent _save_session() call during
export could produce a partial/corrupt read.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The monitor incremented current_session_active_seconds by 1 each loop
then slept 1s, but the ask_stats() call itself takes ~100ms, so each
iteration was ~1.1s. Over a 60-minute session the displayed time was
about 6 minutes fast. Replaced the tick counter with time.monotonic()
so elapsed time is measured against wall clock, not loop iterations.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The two functions were identical except for the limit slice and a missing
lock on the full variant. Unified into a single function with an optional
limit parameter (None = no limit).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
KMH_TO_MPH and KM_TO_MI were the same value (0.621371); replaced all
uses with KM_TO_MI. Also removed the outdated comment on the logging
setup that described an intent already completed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The DOM manipulation block that renders "Server is shutting down" was
copy-pasted verbatim across active_session, paused_session, and
start_session. Extracted into showShutdownOverlay() in base.html;
all three templates and the Close button handler now call it.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Routes that mutate device state (/start, /pause, /resume, /end_session,
speed controls) accepted GET requests, meaning browser prefetch, back
navigation, or link previews could accidentally trigger belt commands.
Changed all action routes to POST-only and updated templates from
<a href> links to <form method="post"> buttons.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ssion sleep

- Removed /manual_reconnect duplicate route; connecting.html now uses
  url_for('reconnect') exclusively
- Renamed run.py's 'startupinfo' dict (which shadowed the meaning of
  subprocess.STARTUPINFO) to popen_kwargs; folded preexec_fn into it
- Removed time.sleep(0.5) from end_session that followed a fire-and-forget
  coroutine dispatch, giving a false impression of synchronisation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sys.exit() was replaced with os._exit() in the graceful shutdown
implementation; the sys import has been dead since then.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
current_speed_kmh was not reset in start_session(), so if a session was
ended mid-walk the previous speed lingered. The next Start would set
belt_running=True, the device would send a speed=0 packet while the belt
was still spinning down, and the auto-pause condition
(belt_running AND speed==0 AND current_speed_kmh>0) would fire immediately
— pausing the new session before the belt had a chance to start.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
A. start_session() now sets _resume_grace_deadline = now+7, matching
   resume_session(). Without it, belt startup jitter (a momentary
   speed=0 report after spin-up begins) could auto-pause a brand new
   session.

B. The chained assignment `current_distance_km = current_steps =
   current_calories = 0.0` in both start_session() and end_session()
   was silently coercing current_steps (an int) to float 0.0, causing
   the history record to store steps as 0.0. Split into separate
   assignments.

C. start_session() now returns early if session_active is already True,
   preventing a direct POST to /start from silently discarding an
   in-progress session without saving it.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
A. _start_belt_sequence was calling start_belt() directly without the
   STANDBY→MANUAL wakeup that _resume_belt_sequence uses. After
   stop_belt() the device likely enters STANDBY, so start_belt() alone
   silently fails. Added the same switch_mode(STANDBY)→switch_mode(MANUAL)
   sequence before start_belt() to match resume behaviour.

B. current_speed_kmh was not reset in end_session(), leaving the last
   belt speed in the global until the device next reports speed=0. Added
   reset to 0.0 alongside the other counter resets.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant