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

Add RawFrame to autogen_headers #124

Merged
merged 4 commits into from
Jan 5, 2025
Merged

Conversation

mcm001
Copy link
Contributor

@mcm001 mcm001 commented Jan 1, 2025

@mcm001
Copy link
Contributor Author

mcm001 commented Jan 1, 2025

Why is wpi__RawFrame.hpp trying to include rpygen/wpi__WPI_RawFrame.hpp? I see ./subprojects/robotpy-wpiutil/wpiutil/rpy-include/rpygen/wpi__RawFrame.hpp locally. but that doesn't match the name. Where's the WPI_ prefix coming from?

@auscompgeek
Copy link
Member

It's seeing the WPI_RawFrame base class and generating an include for the base class' trampoline.

@mcm001
Copy link
Contributor Author

mcm001 commented Jan 1, 2025

Oh interesting. Do I have any way around that? I could just add manual pybind code to the module definition in main.cpp, since it looks like that's true even if I try to add inline code? Or what should I do?

@auscompgeek
Copy link
Member

You could try adding force_no_trampoline: true to the YAML for that class. But looking at the YAML the build spits out in its warning I'm not entirely sure that'll work. That's why I suggested just ignoring all the items we don't need.

@mcm001
Copy link
Contributor Author

mcm001 commented Jan 1, 2025

How would I get robotpy to ignore everything except for the WPI_TimestampSource enum? Can I do that in the class' YAML?

@mcm001
Copy link
Contributor Author

mcm001 commented Jan 1, 2025

Looks like spam ignore: true?

@auscompgeek
Copy link
Member

Oh right, just remembered we have ignored_bases if we do want to expose the RawFrame class.

@mcm001
Copy link
Contributor Author

mcm001 commented Jan 1, 2025

I don't think it makes sense to quote yet. RawFrame is a thin wrapper around a C array of image data - i'm not sure there's a great way (or even a reason yet?) to wrap it in a numpy array for use with opencv stuff. Might be cool to think about in the future though!

Either way, if this is merged, CI on wpilibsuite/allwpilib#7609 should become green :)

@mcm001

This comment has been minimized.

@virtuald
Copy link
Member

virtuald commented Jan 3, 2025

Looks like spam ignore: true?

That's fine, but in the future there's also defaults.ignore:

defaults:
  ignore: true


enums:
WPI_PixelFormat:
ignore: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guessing not worth exposing this enum to Python, since there's an equivalent C++ enum in cscore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I read it yes, the cscore side enum is what the rest of cscore uses in its public api, not the RawFrame version.

Copy link
Member

@auscompgeek auscompgeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@auscompgeek auscompgeek merged commit d2e1864 into robotpy:main Jan 5, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants