report inactivity to server

[alexstrangeparts]

report inactivity to server after a length of time has passed without a button command message

Corresponds to https://github.com/strangeparts/marionette/pull/14

[Copilot]

Pull request overview

Adds an inactivity “heartbeat”/signal from the controller to the server when no button commands have been received for a configurable amount of time, aligning with marionette PR remotv#14.

Changes:

• Added networking.reportInactivity() to emit an INACTIVITY websocket event.
• Added inactivity_timeout to the sample controller configuration.
• Tracked last command time in controller.py and added a periodic inactivity check in the main loop.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
networking.py	Adds a helper to send an INACTIVITY event over the websocket.
controller.sample.conf	Documents/configures an inactivity timeout value.
controller.py	Tracks last BUTTON_COMMAND time and triggers inactivity reporting from the controller loop.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

As written, once the inactivity timeout is exceeded this will call networking.reportInactivity() every second, which can spam the server. Consider reporting only once per inactivity period (e.g., track last_inactivity_report or reset inactive_time after reporting) and/or schedule the check at a lower frequency.

[Copilot]

Initializing inactive_time to 0 means the inactivity condition will be immediately true after startup (once the time.time() call is fixed), potentially reporting inactivity before the robot has even validated/received any command. Initialize it to time.time() or None and skip the inactivity check until it has been set on ROBOT_VALIDATED/first activity.

Suggested change

	inactive_time = 0
	inactive_time = time.time()

[Copilot]

robot_config.getint('robot', 'inactivity_timeout') will raise if existing controller.conf files don’t have this new option. To keep backward compatibility, guard with has_option(...) and use a sensible default (or fallback if available in the ConfigParser version you target).

[Copilot]

reportInactivity() calls webSocket.send(...) unconditionally. If the socket isn’t connected/authenticated (or reconnecting), this can raise and potentially crash the controller loop. Consider checking authenticated/webSocket is not None and/or handling send exceptions inside this helper.

Suggested change

	webSocket.send(json.dumps(
	{"e": "INACTIVITY",
	"d": {}
	}))
	# Ensure websocket is available and authenticated before sending.
	try:
	if not authenticated:
	log.warning("Cannot report inactivity: websocket not authenticated")
	return
	except NameError:
	# 'authenticated' flag not defined yet; treat as not ready.
	log.warning("Cannot report inactivity: authentication state unknown")
	return
	try:
	if webSocket is None:
	log.warning("Cannot report inactivity: websocket not initialized")
	return
	except NameError:
	# 'webSocket' has not been created yet.
	log.warning("Cannot report inactivity: websocket not available")
	return
	try:
	webSocket.send(json.dumps(
	{"e": "INACTIVITY",
	"d": {}}
	))
	except Exception as exc:
	# Avoid crashing the controller loop on send failures.
	log.warning("Failed to report inactivity over websocket: %s", exc)

[Copilot]

This inline comment will likely be included in the value string by ConfigParser() (no inline_comment_prefixes configured), so getint('robot','inactivity_timeout') can fail to parse it. Prefer moving the comment to its own line above (or update ConfigParser construction to support inline comments).

Suggested change

	inactivity_timeout=30 # Time after which, the robot reports inactivity (in seconds)
	# Time after which, the robot reports inactivity (in seconds)
	inactivity_timeout=30

[Copilot]

time.time is a function; time.time.now() will raise AttributeError. Use time.time() (or compute now = time.time() once per loop) for the inactivity comparison.

Suggested change

	if inactive_time + robot_config.getint('robot', 'inactivity_timeout') < time.time.now():
	if inactive_time + robot_config.getint('robot', 'inactivity_timeout') < time.time():

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The method call time.time.now() is incorrect. The time module uses time.time() (a function call), not time.time.now() (which would be a method on a datetime object). This will raise an AttributeError at runtime because time.time is a function object that has no now attribute. Change this to time.time() to match the usage in lines 242 and 277.

Suggested change

	if inactive_time + robot_config.getint('robot', 'inactivity_timeout') < time.time.now():
	if inactive_time + robot_config.getint('robot', 'inactivity_timeout') < time.time():

[Copilot]

The inactivity check runs every second in the main loop, but once the timeout is reached, reportInactivity() will be called repeatedly every second indefinitely until a new BUTTON_COMMAND is received. This could flood the server with inactivity reports. Consider adding a flag to track whether inactivity has already been reported, and reset it when inactive_time is updated, to ensure the report is only sent once per period of inactivity.

[Copilot]

If the inactivity_timeout configuration option is not present in the user's config file (e.g., if they haven't updated their controller.conf from an older version), this will raise a NoOptionError exception and crash the controller. Consider using robot_config.has_option('robot', 'inactivity_timeout') to check if the option exists first, or provide a default value using robot_config.getint('robot', 'inactivity_timeout', fallback=30) to ensure backward compatibility.

Suggested change

	if inactive_time + robot_config.getint('robot', 'inactivity_timeout') < time.time.now():
	if inactive_time + (robot_config.getint('robot', 'inactivity_timeout') if robot_config.has_option('robot', 'inactivity_timeout') else 30) < time.time.now():