gcode: expose status with available commands

[pedrolamas]

This exposes a new gcode status object, with a list of the currently available commands:

It currently exposes the commands (all currently available commands in Klipper), and for each command, if the help text is available, it will also be included.

I understand that we already have the gcode/help endpoint, but this works on a pull, whereas status updates are pushed to the client.

[Sineos]

Would it be possible to programmatically also expose the valid arguments a command takes?

The nifty autocompletion works well for the base command, but often needs back referencing to the documentation to get the arguments right.

[pedrolamas]

I don't think there is anything in Klipper right now that would enable us to provide that information, however, it is something that @meteyou and I have been discussing...

I believe that is out-of-scope for this PR, but it is something we could probably look at to improve the data returned on Klipper gcode/help endpoint

[KevinOConnor]

Thanks. In general it seems useful to me. Some comments:

• Is it necessary to export "ready_handlers"? That seems to be a "low level detail". Could the code instead export "handlers" and "base_handlers"?
• I'm a little concerned about performance of this change (as the lists could get large and rebuilding them every status update seems a bit silly). What about adding a new rebuild_status() method that is called whenever the xxx_gcode_handlers variables are changed and builds a new self.status_info dictionary. That way get_status() could just return self.status_info.

@Arksine - do you have any thoughts on this PR?

-Kevin

[pedrolamas]

Thank you for your comments @KevinOConnor.

On point 1, I'm not entirely sure if this is what you mean there but I've now replaced ready_commands with just commands.

On this topic, I am now considering if we even need both lists or just whatever is currently active and nothing else... @meteyou any comments on this?

On point 2, I added a new _get_status_info object to cache the value to return.

[meteyou]

I talked to @pedrolamas in more detail about this PR. Primarily because of the FR of @Sineos we could make the first step for this feature with this PR (He also wrote this feature already in a Mainsail FR). Currently, we can't deal with attributes of G-Codes in the guis because they are not known. Here, we could provide the right structure to add the rest of this feature later in a second PR.

Furthermore, I see no advantage in separating commands and base commands. This difference makes no difference for us in the GUI. As long as the macro is in the list, it is available.

So, a possible basic structure for this feature would now be our following suggestion:

{
  "result": {
    "status": {
      "gcode": {
        "FIRMWARE_RESTART": {
          "description": "Restart firmware, host, and reload config"
        },
        "RESPOND": {
          "description": "Echo the message prepended with a prefix"
        },
      }
    }
  }
}

After that, it could be expanded to the following in the next PR:

{
  "result": {
    "status": {
      "gcode": {
        "FIRMWARE_RESTART": {
          "description": "Restart firmware, host, and reload config",
          "attributes": []
        },
        "RESPOND": {
          "description": "Echo the message prepended with a prefix",
          "attributes": ["MSG", "TYPE"]
        },
      }
    }
  }
}

Of course, you could simultaneously pursue the second expansion, provide all the necessary information from the attributes, and then create the G-Code page automatically. But it's up to you how far you want it or how far we can expand it. In the end, we can save the endpoint gcode/help in the long term, as it no longer offers any advantage, or at least it is too static for us.

What do you think about this suggestion?

[Arksine]

From what I gather the idea is to have the list of gcode handlers pushed via get_status() so clients are notified when the command list changes? I can see how that would be useful. I agree with Meteyou that there isn't a need to differentiate between ready commands and base commands. API clients (and by extension Moonraker's clients) don't need to use the base gcode commands, there are endpoints that expose most of that functionality (estop, restart, info, etc).

I'm not sure help or parameters should be included here since they are static. Reporting parameters is something we would need to put our heads together to determine the best implementation. I expect it would be a rather significant change.

[pedrolamas]

Following up on the comments, I've updated the PR to only have a single commands returns with whatever are the currently available Klipper commands.

I have not added the descriptions at this time, as I believe we will need to discuss this further in order to also get the parameters in there.

[KevinOConnor]

So, a possible basic structure for this feature would now be our following suggestion:

Thanks. I don't have a particular preference on layout. One thing I'd recommend though, is to put the commands in an additional layer underneath the "gcode" object - something like: printer['gcode']['commands']['FIRMWARE_RESTART']. Otherwise it is very hard to add future status information to the "gcode" object without causing confusion. Just putting a list under printer['gcode']['commands'] (as this PR currently does) also seems fine to me.

Along the same line, it's fine with me if we want to move away from the "gcode/help" endpoint in favor of the "status" mechanism. Since this data is large and basically static, purely for performance reasons, it may make sense to separate it out into its own dict - something like: printer['gcode']['help']['FIRMWARE_RESTART'].

Again, I don't have a strong preferences on this.

Cheers, -Kevin

[KevinOConnor]

Similarly, for what it is worth, I'm fine with exporting parameter information. We'll need to figure out how we want to gather that information internally within the Klipper host code.

-Kevin

[pedrolamas]

Following up on the comments, I have now updated the structure to use a dictionary and include the command help text when available.

As it stands, it will be easy to add parameters to this structure - when we have that capability!

Please check the initial post for a screenshot of the proposed data structure.

[KevinOConnor]

Thanks.

The latest code is similar to meteyou's proposal at https://github.com/Klipper3d/klipper/pull/6393#issuecomment-1818004649 . That's fine with me. Note though, that any change at all to the printer.gcode.commands dict will result in that entire dict being retransmitted to clients subscribed to it. So, for example, if the state transitions from "ready" to "shutdown", the entire command dict will be resent. Placing the static help information in another entry (eg, printer.gcode.help) would likely reduce the amount of traffic (as the static information doesn't update frequently). Again, I'm fine with it either way.

-Kevin

[pedrolamas]

Thank Kevin. Following up on your comments, lets for a second assume we split the information like this:

"result": {
  "commands": ["ACTIVATE_EXTRUDER", "BED_MESH_CALIBRATE", ...],
  "help": {
    "ACTIVATE_EXTRUDER": "Change the active extruder",
    "BED_MESH_CALIBRATE": "Perform Mesh Bed Leveling",
    ...
  }
}

I understand there will be no change in "help" when transitioning from base_gcode_handlers to ready_gcode_handlers and vice/versa, but any call to register_command() method will require a change in both "commands" and "help".

For example, the Manual Probe tool registers the "ACCEPT" and "ABORT" commands (thus requiring a refresh of "commands" and "help") and removes these when the tool finishes (so another refresh of both objects), so I fail to see the advantage of having a "commands" with separate "help".

I too have no preference on this and will be fine with either solution - performance is key in my book!

[pedrolamas]

I think I see now my error: we don't remove the entry from gcode_help when we remove the command...

However, the descriptions can change (so an update would be required):

[pedrolamas]

The current PR approach uses a single element with the active commands and their metadata:

{
    "gcode": {
        "commands": {
            "[COMMAND1]": {
                "help": "[HELP]",
                "params": {
                    "[PARAM1]": "[HELP]",
                    "[PARAM2]": "[HELP]"
                }
            },
            "[COMMAND2]": {
                "help": "[HELP]",
                "params": {
                    "[PARAM1]": "[HELP]",
                    "[PARAM2]": "[HELP]"
                }
            }
        }
    }
}

This means that ANY changes on the active commands or their metadata will force a full refresh of the "commands" element above.

An alternative approach of separating the commands from their metadata could be this:

{
    "gcode": {
        "commands": [
            "[COMMAND1]",
            "[COMMAND2]"
        ],
        "help": {
            "[COMMAND1]": {
                "help": "[HELP]",
                "params": {
                    "[PARAM1]": "[HELP]",
                    "[PARAM2]": "[HELP]"
                }
            },
            "[COMMAND2]": {
                "help": "[HELP]",
                "params": {
                    "[PARAM1]": "[HELP]",
                    "[PARAM2]": "[HELP]"
                }
            }
        }
    }
}

Here "commands" is just a string array, and the rest of the metadata in the "help" element.

The advantage of the above is that "help" is mostly static and only changes when a new command is registered OR we re-registered a command with a different description or parameters (this does happen with the ACCEPT command as it changes description between tools - though I guess we could make these match?)

@Arksine @meteyou any comments on this?

[meteyou]

@pedrolamas Is it even possible to switch from one function with ACCEPT to the next without removing ACCEPT? If not, then the help text should also be changed, as the help text is exchanged when "unload" and "load" the command again.

[pedrolamas]

in terms of help object, the "ACCEPT" is never removed so we are good there.

However, the help text can change between tools!

[meteyou]

ok. i have just double-checked this (with mainline klipper branch). as long as i do not execute a command that i can confirm/cancel with accapt/abort, there is no accect/abort command in /printer/gcode/help. when i execute one command with these commands (PROBE_CALIBRATE as an example), klipper adds ACCEPT/ABORT to the list. unfortunately, these are not removed after i have canceled the command (ABORT). it would be also possible that there is an error in the /printer/gcode/help api, as the ACCEPT command no longer exists (see screenshot).

[KevinOConnor]

If there are any concerns with this PR, please let me know. Otherwise I'll look to merge it in the next few days.

-Kevin

[KevinOConnor]

Thanks.

-Kevin