fix(lib): Remove duplicate method binding (instance/static) from python lib for task#608
fix(lib): Remove duplicate method binding (instance/static) from python lib for task#608
Conversation
|
✅Static analysis result - no issues found! ✅ |
There was a problem hiding this comment.
Pull request overview
This PR fixes a runtime import failure in the Python bindings (lib/python_bindings) caused by a name collision between an instance method and a static method both registered as get_id on the Task class. The fix removes the redundant def_static("get_id", ...) binding and also adds the previously missing Logger::get_verbosity binding.
Changes:
- Removes the duplicate static
Task::get_id(const Task&)binding frompybind_espp.cppthat collided with the instanceget_id()and caused the module to fail on import. - Adds the missing
Logger::get_verbositybinding (both inpybind_espp.cppand__init__.pyi). - Updates
__init__.pyitype stubs to reflect the above changes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
lib/python_bindings/pybind_espp.cpp |
Removes duplicate def_static("get_id") for Task; adds get_verbosity binding for Logger |
lib/python_bindings/espp/__init__.pyi |
Adds Logger.get_verbosity stub; removes Pid.set_config, Pid.clear, and Pid.update stubs (incorrectly) |
One issue was identified:
The set_config, clear, and update methods were removed from the Pid class in __init__.pyi (lines 2159–2164 region), but these methods are still registered in pybind_espp.cpp at lines 1588–1598. This means the type stubs no longer reflect the actual runtime API for Pid: calling pid.set_config(...), pid.clear(), or pid.update(error) is valid at runtime but will produce type-checker/IDE errors. These method stubs need to be restored.
Additionally, the @staticmethod @overload def get_id(task: Task) stub in __init__.pyi for Task (lines 2998–3008, unchanged region) was not removed to match the removal of the corresponding binding in pybind_espp.cpp, leaving the stubs inconsistent with the actual bindings. This is outside the changed region of the diff, so no comment was filed, but it should be addressed alongside the fix.
Description
esp::Task::get_id(...)is not included in the python bindings, as that will fail at runtime when trying to import.Motivation and Context
Fixes an issue with the python bindings which would fail to import, despite being able to build.
How has this been tested?
task.pytest script and ensure it works properly.Screenshots (if appropriate, e.g. schematic, board, console logs, lab pictures):
Types of changes
Checklist:
Software
.github/workflows/build.ymlfile to add my new test to the automated cloud build github action.