Skip to content

Fix Invoke message defaults - #26

Open
cyberman wants to merge 1 commit into
coolcoder613eb:mainfrom
Be-Quiet-Home:fix-invoke-message-defaults
Open

Fix Invoke message defaults#26
cyberman wants to merge 1 commit into
coolcoder613eb:mainfrom
Be-Quiet-Home:fix-invoke-message-defaults

Conversation

@cyberman

Copy link
Copy Markdown

Summary

  • use Python None as the default for optional BMessage* arguments in BButton.Invoke() and BInvoker.Invoke()
  • add a regression test covering both bindings

Background

Both methods accept an optional native BMessage*:

status_t Invoke(BMessage* message = NULL)

The bindings registered the pointer default as C++ NULL:

py::arg("message") = NULL

Pybind11 exposed this as the Python integer 0:

BButton.Invoke(message: BMessage = 0)
BInvoker.Invoke(message: BMessage = 0)

Calling either method without an argument therefore failed during argument conversion:

TypeError: Invoke(): incompatible function arguments

Passing None explicitly was accepted, confirming that the pointer conversion itself was valid and only the registered default was incorrect.

Fix

Register the optional pointer arguments using Python None:

py::arg("message") = py::none()

The resulting Python signatures now correctly advertise:

BButton.Invoke(message: BMessage = None)
BInvoker.Invoke(message: BMessage = None)

Validation

  • reproduced the omitted-argument failure for both bindings
  • verified that explicit None converts correctly
  • used Python trampoline subclasses so the native invocation mechanism was not executed during regression testing
  • rebuilt Button.so and Invoker.so
  • verified omitted arguments reach both trampolines as None
  • verified explicit BMessage arguments remain compatible
  • verified both generated Python signatures advertise None instead of 0
  • independently applied the patch against upstream main at 99940f7620ce6860bdb993922308079160e4fbad
  • independently reproduced both module artifacts
  • independently reran the regression, signature, and explicit-message compatibility tests successfully

@Zardshard

Copy link
Copy Markdown
Collaborator

Hello! Since we haven't used AI to this point in contributing, we've had to clarify our stance on this. Given that these commits are pretty easy to independently review and implement, we're treating these as issue reports for now. That will avoid this repository having AI-generated code in it, at least for the time being.

If you plan on implementing more difficult changes (such as getting rid of those py::nodelete's in favor of something that's actually properly memory managed), then we will need to figure out our stance on AI further. If we decide against it, then you can obviously fork this repository if you want. Good luck, and thanks for the PRs!

@cyberman

Copy link
Copy Markdown
Author

Thx for clarifying.

My changes are small, narrowly scoped, and independently reviewable. The goal is not to redesign Haiku-PyAPI or undertake broader ownership changes such as replacing the existing py::nodelete model.

I only need a stable and predictable Haiku-PyAPI base for Printrun. The patches address concrete defects encountered during that work.

For future findings, please let me know whether you prefer a pull request with the tested fix or an issue report without a patch.

@coolcoder613eb

Copy link
Copy Markdown
Owner

What is Printrun?

@cyberman

Copy link
Copy Markdown
Author

Printrun is a set of host tools for 3D printers, best known for Pronterface and Pronsole.

I'm working on a native Haiku frontend using Haiku-PyAPI instead of wxPython. That's how I ran into these binding bugs.

@Zardshard

Copy link
Copy Markdown
Collaborator

I'm working on a native Haiku frontend using Haiku-PyAPI instead of wxPython. That's how I ran into these binding bugs.

Ah, nice!

For future findings, please let me know whether you prefer a pull request with the tested fix or an issue report without a patch.

Hmm, I guess an issue report would work better. This raises some funny philosophical issues. Let's take a dive into some of the mild absurdity, shall we? 😉

When exactly is a contribution made by AI? If we were to manually fix these bugs, they would have a good chance of being the exact same as what the AI would have done, excluding the test application that you've included in your PRs. Since our code is indistinguishable from an AI contribution, was it secretly just written by AI, only with extra steps?

Say you wrote an issue and gave an AI-assisted test program that reproduces the issue. We then run it and realize the problem and write the code to fix it. This happens to match exactly with what your AI wrote. Was our contribution actually made by AI? 😅 It becomes worse if you include a strong hint as to what the underlying problem is.

Oh well, since we currently are unlikely to merge the pull requests, you making issues makes more sense. But as to what exactly you think should go into those issues I'll leave up to you. Maybe just a test program that reproduces the problem? Maybe also a strong hint as to what is causing the problems? Maybe just a minimal description of the bug? Whatever you decide, thank you for your contributions!

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.

4 participants