In the MenuManager `key_event` method, add support for event notifications

[balthisar]

In the MenuManager key_event method, add support for event notifications for key events using the send_event() method. This will allow other host modules to register for event notifications with register_event_handler().

This could be useful for developers to know when the rotary encoder is activated so that they might do something such as activate LED lights.

The change publishes two notifications for each key event:

• menu:{key} notification can be subscribed to if the developer is only interested in a single key event type, such as clicking the encoder. One could subscribe to every event type with subsequent uses of register_event_handler(), one for each key event type.

• menu:action notification can be subscribed to in order to receive a notification for every key event type, avoiding the need to use multiple register_event_handler() uses. The actual key pressed will be present in the next positional parameter. One may still prefer to receive a single notification with the first version in order to avoid excessive use of the callbacks if only one event is interesting.

Signed-off-by: Jim Derry [email protected]

[balthisar]

Given that the minimum version of Python for Klipper is 3.8, and f-strings were introduced in 3.6, I'm not sure if I should worry about the failing check above. I'd be happy to switch the the more verbose C printf style string interpolation if preferred.

[nefelim4ag]

I don't know where you found that Klipper supports Python >= 3.8. But it still supports Python 2.7. So, any Python code should have correct Python 2.7 syntax. So, the tests should not fail.

Hope that helps.

[balthisar]

My Google-fu must be off today, but yes, that helps. I've made the change accordingly. And the build is successful now.

[balthisar]

Results of self-review per the CONTRIBUTING.md:

1. Is the submission free of defects and is it ready to be widely deployed?

Yes. The submission adds two lines of existing internal API in order to allow additional event notifications. Internal API is not experimental, and regression testing has passed.

2. Does the submission provide a "high impact" benefit to real-world users performing real-world tasks?

No, but: Developers that take advantage of the change can develop Extras that do provide a "high impact" benefit to real-world users. For example, capturing the menu events can be used to turn on lights during interactivity automatically. As this is a developer-facing change and not a user-facing change, impact would only be measured by additional Extras that developers can imagine (I do have an "Extra" that depends on this PR; it's not submitted for PR, but I could estimate its impact as a proxy for this change if desired).

3. Is the copyright of the submission clear, non-gratuitous, and compatible?

N/A; original source copyright remains in effect.

4. Does the submission follow guidelines specified in the Klipper documentation?

Yes.

5. Is the Klipper documentation updated to reflect new changes?

N/A for this change.

6. Are commits well formed, address a single topic per commit, and independent?

Yes, especially given the limited source code changes. Commits are all signed-off.

[KevinOConnor]

Thanks. However, any internal change to the code in this repo must come with some kind of user facing improvement (for those running the code in this repo). So, if you're looking to propose some kind of new module then I would suggest putting the commit on this PR as part of that submission.

For what it is worth though, we rarely extend the current lcd display code. We generally recommend using a frontend that uses the API server (eg, fluidd, mainsail, moonraker, etc.).

-Kevin

[balthisar]

Thanks, @KevinOConnor. I actually have a user-facing Extra that depends on this change, and I'd be happy to add it to this PR (actually, I would close this PR and create a new one). That brings significant extra work, which I'm happy to do, if you think that the functionality is something that's likely to be merged once it's in an acceptable state. That is, I'd rather not work on the "complete package" if I knew ahead of time there was no interest.

Current package that's totally NOT in a PR-ready state: https://github.com/balthisar/klipper_on_unidle — it adds an unidle event that will run gcode when the printer transitions back to Ready. This PR, is used by the module so that if the user uses the display, then the printer can transition back to Ready, too.

As such, it's not really a change to the lcd display code as much as it is a method to detect that the LCD knob is being used.

If there's no interest in the on_unidle, then I could refactor this into a user-facing change simply by bringing the printer back to Ready from Idle if the display knob is used. I think that could be considered user-facing because it re-enables idle timeout. We'd probably not want that behavior to change by default, so perhaps adding a new setting to idle_timeout such as display_wakes_printer or similar.

Thoughts?

[nefelim4ag]

Jfyi: Looks similar to the: https://klipper.discourse.group/t/adding-on-ready-gcode-to-idle-timeout-py-vs-new-host-module/23028

[github-actions[bot]]

Thank you for your contribution to Klipper. Unfortunately, a reviewer has not assigned themselves to this GitHub Pull Request. All Pull Requests are reviewed before merging, and a reviewer will need to volunteer. Further information is available at: https://www.klipper3d.org/CONTRIBUTING.html

There are some steps that you can take now:

1. Perform a self-review of your Pull Request by following the steps at: https://www.klipper3d.org/CONTRIBUTING.html#what-to-expect-in-a-review If you have completed a self-review, be sure to state the results of that self-review explicitly in the Pull Request comments. A reviewer is more likely to participate if the bulk of a review has already been completed.
2. Consider opening a topic on the Klipper Discourse server to discuss this work. The Discourse server is a good place to discuss development ideas and to engage users interested in testing. Reviewers are more likely to prioritize Pull Requests with an active community of users.
3. Consider helping out reviewers by reviewing other Klipper Pull Requests. Taking the time to perform a careful and detailed review of others work is appreciated. Regular contributors are more likely to prioritize the contributions of other regular contributors.

Unfortunately, if a reviewer does not assign themselves to this GitHub Pull Request then it will be automatically closed. If this happens, then it is a good idea to move further discussion to the Klipper Discourse server. Reviewers can reach out on that forum to let you know if they are interested and when they are available.

Best regards, ~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.