Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BREAKING] Move and update many component implementations to support litgen-based pybind11 generation to overhaul python library #291

Merged
merged 86 commits into from
Aug 11, 2024

Conversation

finger563
Copy link
Contributor

@finger563 finger563 commented Jul 12, 2024

Description

  • Use litgen to generate the python bindings instead of manually writing them (NOTE: because of some bugs in litgen and the underlying srcml, some manual modifications are required after generation)
  • Refactor code to move implementation from header into source since litgen has trouble parsing header files that contain implementation.
  • Refactor code to adjust how various type are initialized since litgen has trouble with template type initialization of the form T(0)
  • Update code for default initialization of certain types since litgen has trouble generating bindings for certain types of structs
  • Refactor implementation which cannot be moved from header files to remove if () conditionals since that seems to break litgen parsing
  • Update socket code to support winsock library and use on windows
  • Add c++ pc tests for udp_client and udp_server
  • Add CI for building library on macos/linux/windows

[BREAKING CHANGES]

  1. espp::Gaussian was updated to remove float gamma() const and void gamma(float) in favor of making gamma public (same for beta and alpha). This was done because the existing interface was not supported by litgen since it involved overloads of the same function name, so a breaking API change was necessary, and making them public seemed to be better than changing the methods to get_* and set_*.
  2. espp::Task::run_on_core has been moved outside the Task class and into its own header run_on_core.hpp, since it must be implemented in a header file, but cannot be parsed by litgen. It is now within the espp::task namespace as espp::task::run_on_core.
  3. espp::detail::TcpTransmitConfig was moved into the espp::TcpSocket class to become espp::TcpSocket::TransmitConfig. This was to simplify use of litgen to not have to expose the espp::detail namespace and to simplify the code, using an alternate workaround for the gcc bug that originally required the use of the detail namespace - instead we now have a Defaults() method that we use for function default arguments.

NOTE:
Right now the autogenerated bindings have to be modified since:

  • Template classes with config structs such as template <typename T> espp::RangeMapper's espp::RangeMapper::Config are not handled generated by litgen properly, they result in:
    • Loss of template parameter specification
    • Binding to the wrong specialization class binding
  • Vector2d specializations are not always generated with the appropriate specialization
  • constructors in classes with config structs are not generated properly

See:

Motivation and Context

Supporting an interface via python currently requires a lot of manual binding writing, but there are some newer tools which can help offload some or all of this work by parsing the headers and generating the bindings.

We also would like to automatically build the library in CI to ensure it is kept up to date.

How has this been tested?

Generating the python bindings and building the c++ / python libraries

From within lib:

  1. Configuring the python virtual environment
  2. python3 -m venv env
  3. source env/bin/activate
  4. pip install -r requirements.txt
  5. running python autogenerate_bindings.py
  6. running pre-commit run --all-files
  7. Modifying the generated python_bindings/pybind_espp.cpp
  8. running ./build.sh

Building and running all the affected examples:

CleanShot 2024-07-30 at 08 17 41

Testing python udp_client and c++ udp_server on windows (in parallels VM on MacOS):
CleanShot 2024-08-10 at 22 17 26@2x

Building and running all the c++ tests inside pc

  1. cli
  2. task
  3. timer
  4. udp_client
  5. udp_server

Running all the python tests inside python

  1. task
  2. timer
  3. udp_client
  4. udp_server

Screenshots (if appropriate, e.g. schematic, board, console logs, lab pictures):

PC Tests:

CleanShot 2024-07-30 at 08 20 35

Python Tests:

CleanShot 2024-07-30 at 08 22 49

Testing python using Windows library built in CI and running in a parallels windows VM:

CleanShot 2024-08-03 at 19 54 52@2x

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update
  • Hardware (schematic, board, system design) change
  • Software change

Checklist:

  • My change requires a change to the documentation.
  • I have added / updated the documentation related to this change via either README or WIKI

Software

  • I have added tests to cover my changes.
  • I have updated the .github/workflows/build.yml file to add my new test to the automated cloud build github action.
  • All new and existing tests passed.
  • My code follows the code style of this project.

@finger563 finger563 self-assigned this Jul 12, 2024
@finger563 finger563 marked this pull request as draft July 12, 2024 02:12
Copy link

github-actions bot commented Jul 12, 2024

✅Static analysis result - no issues found! ✅

…tead of having getter/setter functions for them with the same name
…task, timer, socket, tcpsocket, udpsocket and fixed autogenerated bindings. trying to get autogeneration to work for template classes
…cilitate proper binding generation; remove TcpTransmitConfig from detail namespace and update it to be TcpSocket::TransmitConfig with alternate workaround for gcc bug; updated ftp and rtsp accordingly
…t of manual editing of the generated bindings file, but it is way better than having to write the whole thing from scratch
@finger563 finger563 added enhancement New feature or request python labels Jul 29, 2024
@finger563 finger563 merged commit e4f946f into main Aug 11, 2024
66 checks passed
@finger563 finger563 deleted the feature/file-system-cross-platform-update branch August 11, 2024 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lib library for use with other systems / languages litgen python xplat cross platform support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant