Skip to content
This repository was archived by the owner on Nov 13, 2023. It is now read-only.

SmartDashboard.putData fails with custom Sendable subclasses #630

Closed
auscompgeek opened this issue Feb 6, 2020 · 10 comments · Fixed by robotpy/robotpy-wpiutil#40
Closed
Labels

Comments

@auscompgeek
Copy link
Member

auscompgeek commented Feb 6, 2020

robot.py:

import wpilib


class Test(wpilib.Sendable):
    def __init__(self):
        super().__init__()
        wpilib.SendableRegistry.getInstance().add(self, "Test", 1)

    def initSendable(self, builder):
        pass


class Robot(wpilib.TimedRobot):
    def robotInit(self):
        self.test = Test()
        wpilib.SmartDashboard.putData("test", self.test)


if __name__ == "__main__":
    wpilib.run(Robot)

Traceback:

Traceback (most recent call last):
  File "~/.local/share/virtualenvs/rpy-dev/lib/python3.8/site-packages/wpilib/_impl/start.py", line 106, in start
    self.robot.startCompetition()
  File "robot.py", line 16, in robotInit
    wpilib.SmartDashboard.putData("test", self.test)
RuntimeError: return_value_policy = copy, but type frc::SendableBuilderImpl is non-copyable!

It looks like it's the call to initSendable that causes this. If the method is deleted:

Traceback (most recent call last):
  File "~/.local/share/virtualenvs/rpy-dev/lib/python3.8/site-packages/wpilib/_impl/start.py", line 106, in start
    self.robot.startCompetition()
  File "robot.py", line 14, in robotInit
    wpilib.SmartDashboard.putData("test", self.test)
RuntimeError: <__main__.Test object at 0x63b424da04a0> does not override required function "Sendable::initSendable"

From the tracebacks I got, it looks like it throws up in frc::SendableRegistry::Publish.

@auscompgeek auscompgeek added the bug label Feb 6, 2020
@auscompgeek
Copy link
Member Author

I'm quite confused as to what exactly pybind11 is doing here, given that Sendable::InitSendable takes a reference…

@auscompgeek
Copy link
Member Author

I managed to get a native code backtrace:

#1  0x00007ffff34266db in pybind11::detail::type_caster_generic::cast (_src=_src@entry=0xf73fa8, policy=policy@entry=pybind11::return_value_policy::copy, parent=..., parent@entry=..., tinfo=tinfo@entry=0xdc1380,
    copy_constructor=0x0, move_constructor=move_constructor@entry=0x0, existing_holder=0x0) at /home/ubuntu/frc/robotpy/venv/lib/python3.8/site-packages/robotpy_build/pybind11/include/pybind11/cast.h:548
#2  0x00007ffff343fced in pybind11::detail::type_caster_base<frc::SendableBuilder>::cast (parent=..., policy=pybind11::return_value_policy::copy, src=0xf73fa8)
    at /home/ubuntu/frc/robotpy/venv/lib/python3.8/site-packages/robotpy_build/pybind11/include/pybind11/cast.h:902
#3  pybind11::detail::type_caster_base<frc::SendableBuilder>::cast (parent=..., policy=pybind11::return_value_policy::copy, src=...)
    at /home/ubuntu/frc/robotpy/venv/lib/python3.8/site-packages/robotpy_build/pybind11/include/pybind11/cast.h:867
#4  pybind11::make_tuple<(pybind11::return_value_policy)1, frc::SendableBuilder&> (args_#0=...) at /home/ubuntu/frc/robotpy/venv/lib/python3.8/site-packages/robotpy_build/pybind11/include/pybind11/cast.h:1802
#5  0x00007ffff35e6228 in pybind11::detail::simple_collector<(pybind11::return_value_policy)1>::simple_collector<frc::SendableBuilder&> (this=0x7fffffffc2e0)
    at /home/ubuntu/frc/robotpy/venv/lib/python3.8/site-packages/robotpy_build/pybind11/include/pybind11/cast.h:1995
#6  pybind11::detail::collect_arguments<(pybind11::return_value_policy)1, frc::SendableBuilder&, void> ()
    at /home/ubuntu/frc/robotpy/venv/lib/python3.8/site-packages/robotpy_build/pybind11/include/pybind11/cast.h:2139
#7  pybind11::detail::object_api<pybind11::handle>::operator()<(pybind11::return_value_policy)1, frc::SendableBuilder&> (this=0x7fffffffc2b0)
    at /home/ubuntu/frc/robotpy/venv/lib/python3.8/site-packages/robotpy_build/pybind11/include/pybind11/cast.h:2159
#8  rpygen::Pyfrc__Sendable<frc::Sendable>::InitSendable (this=0xf2aae0, builder=...) at /home/ubuntu/frc/robotpy/robotpy-wpilib/wpilib/rpy-include/rpygen/frc__Sendable.hpp:27
#9  0x00007ffff3d2c580 in frc::SendableRegistry::Publish (this=this@entry=0x7ffff3f7bd10 <frc::SendableRegistry::GetInstance()::instance>, sendableUid=<optimized out>,
    table=std::shared_ptr<nt::NetworkTable> (use count 3, weak count 0) = {...}) at /__w/1/s/wpilibc/src/main/native/cpp/smartdashboard/SendableRegistry.cpp:311
#10 0x00007ffff3cbdca4 in frc::SmartDashboard::PutData (key=..., data=0xf2aae0) at /__w/1/s/wpilibc/src/main/native/cpp/smartdashboard/SmartDashboard.cpp:101

@virtuald
Copy link
Member

virtuald commented Feb 15, 2021

I think pybind11 is trying to take ownership of the reference (which is actually a C++ local), I think we would need a proxy of some kind here.

@auscompgeek
Copy link
Member Author

It is rather odd that pybind11 wants to take a copy of SendableBuilder...

@virtuald
Copy link
Member

virtuald commented Feb 15, 2021

Well, after the function is called, what happens to the python reference to SendableBuilder? Since it's a reference, it's potentially not safe for it to continue to exist?

@virtuald
Copy link
Member

Actually, that doesn't make sense. Likely what's happening is...

  • It tries to find an existing python object that references the SendableBuilder
  • Doesn't find it, so it looks at the holder and sees it's a shared_ptr. Since it can't just reference it, it has to copy the instance?

.. that is a bit surprising.

@virtuald
Copy link
Member

I thought about this some more tonight, and while the behavior I cited before is still what I think is likely happening, I'm no longer surprised by it (though it is surprising!).

The ideal solution would be a pybind11 holder type that could be explicitly disowned. There are some hacky PRs in pybind11, but there's a py::smart_holder coming soon that I'm optimistic will enable this functionality to be done easily with a temporary std::unique_ptr. I think. We'll see?

@auscompgeek
Copy link
Member Author

Doesn't find it, so it looks at the holder and sees it's a shared_ptr. Since it can't just reference it, it has to copy the instance?

Note that it still tries to copy it when the SendableBuilder* holders aren't shared_ptr.

@virtuald
Copy link
Member

This is reported to still be broken.

@auscompgeek
Copy link
Member Author

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants