feat(console): pass context to command handlers (IDFGH-16124)

[JVKran]

Description

One is now able to override the statically configured context with esp_console_cmd_register by calling the new function esp_console_run_with_ctx. I've taken a look at the original pull-request for the addition of esp_console_cmd_func_with_context_t and have reused the same context and function.

Specifically, this is very useful when commands are received through (for example) MQTT, HTTP and ESP-NOW. It's great to be able to pass these backends so the command handlers can redirect output to those backends.

Testing

Changes have been tested in our existing code base. Changes are very basic and simple.

---

Checklist

Before submitting a Pull Request, please ensure the following:

• 🚨 This PR does not introduce breaking changes.
• All CI checks (GH Actions) pass.
• Documentation is updated as needed.
• Tests are updated or added as necessary.
• Code is well-commented, especially in complex areas.
• Git history is clean — commits are squashed to the minimum necessary.

[CLAassistant]

All committers have signed the CLA.

[github-actions]

	Messages
📖	🎉 Good Job! All checks are passing!

👋 Hello JVKran, we appreciate your contribution to this project!

---

📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.

---

🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project.

---

Click to see more instructions ...

This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...

We do welcome contributions in the form of bug reports, feature requests and pull requests via this public GitHub repository.

This GitHub project is public mirror of our internal git repository

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved, we synchronize it into our internal git repository.
4. In the internal git repository we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
5. If the change is approved and passes the tests it is merged into the default branch.
5. On next sync from the internal git repository merged change will appear in this public GitHub repository.

Generated by 🚫 dangerJS against 5fd9fb9

[o-marshmallow]

Hello @JVKran ,

If I understand correctly, you want to be able to invoke a command with different parameters (context) than the one provided when registering the function, basically, overriding the default parameter. Is it correct?

[o-marshmallow]

Suggested change

	esp_err_t esp_console_run_with_ctx(void* invocation_ctx, const char *cmdline, int *cmd_ret)
	esp_err_t esp_console_run_with_context(void* invocation_ctx, const char *cmdline, int *cmd_ret)

Since the type is named esp_console_cmd_func_with_context_t, I would suggest to keep context instead of ctx, even

[o-marshmallow]

Suggested change

	*cmd_ret = (*cmd->func_w_context)(invocation_ctx, argc, argv);
	*cmd_ret = (*cmd->func_w_context)(invocation_ctx ?: cmd->context, argc, argv);

I think this is what you meant, else the commands will never ne executed with their default context

[o-marshmallow]

I think it's more a context override rather than additional data

[o-marshmallow]

Since the context is an optional parameter in this function, I think it'd make more sense to put it at the end

[JVKran]

Hi @o-marshmallow, I've just incorporated your feedback. Thanks.

[o-marshmallow]

@JVKran During an internal review, I was asked about the objective behind using a dynamic context. Because in theory, when a command is registered we already know what context it will be receiving when invoking the command via the console. The only dynamic behavior the command have would be through the argv parameters.

And if let's say you have a request coming from HTTP and another other coming from MQTT that is very similar with with different way of replying to (output backends), then why using commands at all rather than regular C functions?

[JVKran]

Hi @o-marshmallow,

The objective is the following;

• Let's establish that a command has a name, arguments, values, a result (actions occuring, values being changed, etc.) and a return value.
• Currently, the handling of output isn't part of the console component, which I find weird. The only output is the return value of the handler.
• Let's say a command named get_value can be called through MQTT, HTTP and ESP-NOW. Currently, there's absolutely no way for the command handler to know where to write the requested value to, other than to print it to UART or USB (while I want the output to be sent to MQTT, HTTP or ESP-NOW).
• I want it to be able to write the output (the requested value) to the source backend.

Practically, in my main task, where MQTT payloads, HTTP requests and ESP-NOW packets are received, I can simply call esp_console_run to execute the command. What I want to establish is that I can provide three functions (with the same signature) as result outputs; esp_console_run_with_context(&esp_now_send, etc.), esp_console_run_with_context(&http_reply, etc.) In the command handler I can check for the function being passed and then simply call context->write(CUSTOM_PAYLOAD, regardless of source backend.

This desired functionality isn't possible when the context is set during registration. The context set during registration is static, while I want it to be dynamic; they serve different purposes.

Best regards,

Jochem

[o-marshmallow]

@JVKran I see why you would need a dynamic context in your case, but the question is why do you need to use a command for this?
The main purpose of the commands is to be used from the ESP console, and so invoked after parsing a command line from string (usually typed by a user).
In your case, it seems to me that using a command is not necessary, why not having custom C functions for that?

[JVKran]

Because I literally need to expose a command line over these interfaces. I have around 30 commands that I need to expose over MQTT, HTTP, ESP-NOW and BLE-SPP. These interfaces must be available to literally do exactly what the otherwise useless esp_console_run provides, minus a dynamic context.

I want these interfaces to be exactly the same to the users as the REPL's on UART and USB are, minus the logging itself. If I wouldn't be able to use this solution, I would need to not have 30 functions (for the command handlers), but 30 times the amount of interfaces. That's ridiculous.

Take Zephyr's shell, for example. They provide a pointer to the shell so one can use shell_printf to print to the source command shell. I have no doubt that many others will be very happy if you were to provide an easier way to register logging backends (not just the printf that can be overriden) and would pass the source shell (the shell through which the command is issued) to the handler. This has always been a shortcoming in my opinion. If not, I feel like this dynamic context is a very reasonable and pragmatic solution. I dislike Zephyr in a lot of ways, but console/shell-wise, it's excellent.

[o-marshmallow]

Thanks for the explanation, I understand what you mean. Can you please apply the changes I mentioned above so that I can open an internal merge request? (If you prefer I can also directly apply the changes myself on top of your PR when merging it internally)

[JVKran]

Hi @o-marshmallow,

I'm happy you understand. I've incorporated all changes you mentioned above and also used the pre-commit hooks. Is there anything else I can/need to assist you with?

Best regards,

Jochem

[o-marshmallow]

sha=5fd9fb9e09b672429362c5be6b40df71424f59d7

[o-marshmallow]

@JVKran, I got some internal feedbacks, the console component is going to be deprecated in IDF since it will be completely refactored, so it wouldn't make a lot of sense to add a new feature to maintain. However, there is a simpler modification that would help you achieve what you need: make find_command_by_name public.

Thanks to it, you will be able to get the registered cmd_item_t command, and thus be able to call its callback with any context you want, that would even allow you to customize argc and argv.

If you think this is acceptable, I will open an internal MR to make find_command_by_name public.