Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 18, 2025

Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.

Original prompt

Replace the vulnerable stats() implementation in klippy/klippy/extras/oams.py with a robust implementation that avoids "not all arguments converted during string formatting" errors. The current stats() uses old %-formatting and a tuple of values which can mismatch and raise TypeError. Change the file to implement a safer stats() function that:

  • Uses .format() (or f-strings) and getattr() to safely pull attributes with defaults.
  • Safely indexes f1s_hes_value and hub_hes_value arrays with bounds checks.
  • Returns (False, msg) where msg is a single formatted string, matching the expected return type.
  • Catches exceptions and logs them, returning (False, "OAMS: stats error") on failure.

Patch details (exact replacement content):

Replace the existing stats(self, eventtime) method with the following implementation:

def stats(self, eventtime):
    """
    Return a tuple (update_ui, formatted_string) for the stats reporter.
    Use .format() and safe attribute access to avoid mismatch errors.
    """
    try:
        oams_idx = getattr(self, "oams_idx", "<idx?>")
        current_spool = getattr(self, "current_spool", None)
        fps_value = getattr(self, "fps_value", 0.0)
        f1s = getattr(self, "f1s_hes_value", [0, 0, 0, 0])
        hub = getattr(self, "hub_hes_value", [0, 0, 0, 0])
        kp = getattr(self, "kp", None)
        ki = getattr(self, "ki", None)
        kd = getattr(self, "kd", None)
        encoder_clicks = getattr(self, "encoder_clicks", 0)
        i_value = getattr(self, "i_value", 0.0)

        msg = (
            "OAMS[{idx}]: current_spool={spool} fps_value={fps:.3f} "
            "f1s_hes_value_0={f10} f1s_hes_value_1={f11} f1s_hes_value_2={f12} f1s_hes_value_3={f13} "
            "hub_hes_value_0={h0} hub_hes_value_1={h1} hub_hes_value_2={h2} hub_hes_value_3={h3} "
            "kp={kp} ki={ki} kd={kd} encoder_clicks={enc} i_value={i:.3f}"
        ).format(
            idx=oams_idx,
            spool=current_spool,
            fps=float(fps_value),
            f10=f1s[0] if len(f1s) > 0 else 0,
            f11=f1s[1] if len(f1s) > 1 else 0,
            f12=f1s[2] if len(f1s) > 2 else 0,
            f13=f1s[3] if len(f1s) > 3 else 0,
            h0=hub[0] if len(hub) > 0 else 0,
            h1=hub[1] if len(hub) > 1 else 0,
            h2=hub[2] if len(hub) > 2 else 0,
            h3=hub[3] if len(hub) > 3 else 0,
            kp=kp,
            ki=ki,
            kd=kd,
            enc=encoder_clicks,
            i=float(i_value),
        )

        return False, msg
    except Exception:
        logging.exception("OAMS: Exception building stats string")
        return False, "OAMS: stats error"

Requirements:

  • Apply this change on branch 'oams-manager-load' and create a pull request targeting the same branch (i.e., update the branch and open a PR so the change can be reviewed). The PR title should be the problem_title and the description should explain that this fixes a TypeError in the stats reporter by replacing %-formatting with safe formatting and attribute access.
  • Make sure to preserve the rest of the file unchanged.

Please create the PR and provide the PR URL in the response.

This pull request was created as a result of the following prompt from Copilot chat.

Replace the vulnerable stats() implementation in klippy/klippy/extras/oams.py with a robust implementation that avoids "not all arguments converted during string formatting" errors. The current stats() uses old %-formatting and a tuple of values which can mismatch and raise TypeError. Change the file to implement a safer stats() function that:

  • Uses .format() (or f-strings) and getattr() to safely pull attributes with defaults.
  • Safely indexes f1s_hes_value and hub_hes_value arrays with bounds checks.
  • Returns (False, msg) where msg is a single formatted string, matching the expected return type.
  • Catches exceptions and logs them, returning (False, "OAMS: stats error") on failure.

Patch details (exact replacement content):

Replace the existing stats(self, eventtime) method with the following implementation:

def stats(self, eventtime):
    """
    Return a tuple (update_ui, formatted_string) for the stats reporter.
    Use .format() and safe attribute access to avoid mismatch errors.
    """
    try:
        oams_idx = getattr(self, "oams_idx", "<idx?>")
        current_spool = getattr(self, "current_spool", None)
        fps_value = getattr(self, "fps_value", 0.0)
        f1s = getattr(self, "f1s_hes_value", [0, 0, 0, 0])
        hub = getattr(self, "hub_hes_value", [0, 0, 0, 0])
        kp = getattr(self, "kp", None)
        ki = getattr(self, "ki", None)
        kd = getattr(self, "kd", None)
        encoder_clicks = getattr(self, "encoder_clicks", 0)
        i_value = getattr(self, "i_value", 0.0)

        msg = (
            "OAMS[{idx}]: current_spool={spool} fps_value={fps:.3f} "
            "f1s_hes_value_0={f10} f1s_hes_value_1={f11} f1s_hes_value_2={f12} f1s_hes_value_3={f13} "
            "hub_hes_value_0={h0} hub_hes_value_1={h1} hub_hes_value_2={h2} hub_hes_value_3={h3} "
            "kp={kp} ki={ki} kd={kd} encoder_clicks={enc} i_value={i:.3f}"
        ).format(
            idx=oams_idx,
            spool=current_spool,
            fps=float(fps_value),
            f10=f1s[0] if len(f1s) > 0 else 0,
            f11=f1s[1] if len(f1s) > 1 else 0,
            f12=f1s[2] if len(f1s) > 2 else 0,
            f13=f1s[3] if len(f1s) > 3 else 0,
            h0=hub[0] if len(hub) > 0 else 0,
            h1=hub[1] if len(hub) > 1 else 0,
            h2=hub[2] if len(hub) > 2 else 0,
            h3=hub[3] if len(hub) > 3 else 0,
            kp=kp,
            ki=ki,
            kd=kd,
            enc=encoder_clicks,
            i=float(i_value),
        )

        return False, msg
    except Exception:
        logging.exception("OAMS: Exception building stats string")
        return False, "OAMS: stats error"

Requirements:

  • Apply this change on branch 'oams-manager-load' and create a pull request targeting the same branch (i.e., update the branch and open a PR so the change can be reviewed). The PR title should be the problem_title and the description should explain that this fixes a TypeError in the stats reporter by replacing %-formatting with safe formatting and attribute access.
  • Make sure to preserve the rest of the file unchanged.

Please create the PR and provide the PR URL in the response.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@lindnjoe lindnjoe marked this pull request as ready for review October 18, 2025 20:55
@lindnjoe lindnjoe merged commit 19f19ad into oams-manager-load Oct 18, 2025
1 check failed
Copilot AI requested a review from lindnjoe October 18, 2025 20:56
Copilot finished work on behalf of lindnjoe October 18, 2025 20:56
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.

2 participants