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

Personal branch #106

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

Personal branch #106

wants to merge 21 commits into from

Conversation

yoyobuae
Copy link
Contributor

No description provided.

Copy link
Owner

@ju1ce ju1ce left a comment

Choose a reason for hiding this comment

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

for the "Draw tracker axis at current location" commit, it draws axis not bassed on previous frame, but rather based on where the driver thinks the tracker is, which i think is a good visualization of smoothing

@funnbot
Copy link
Contributor

funnbot commented Jan 8, 2023

  • String fixes - could you explain what this does?
  • remove git submodule --filter=tree:0 - removed as it was unnecessary just for BridgeDriver
  • Draw tracker axis at current location - Seems like this isn't necessary due to juices comment.
  • Privacy mode for out window
  • Make search window be centered on the center of tracker
  • Set apriltag_detector nthreads to 1 - Added a config option
  • Dont send tracker if more than 100ms have passed since last detected
  • Make named pipe address configurable.
  • Bigger drawImgSize
  • Improved camera calibration delay between snapshots.
  • Use hex numbering for tracker names in driver - Shouldn't we just shorten the ApriltagTracker prefix for tracker names.
  • Draw midpoint of camera preview to help with calibration
  • Scale search window according to tracker distance from camera
  • Fix buggy measurement of time since frame was captured
  • Fix missing null byte when sending IPC commands - This isn't necessary from what I understand, but can be helpful if not using SOCK_SEQPACKET.

@yoyobuae
Copy link
Contributor Author

  • String fixes - could you explain what this does?

I get build errors without those changes

* [ ]  `Draw tracker axis at current location` - Seems like this isn't necessary due to juices comment.

It is just super weird to use drawAxis like that. Normally the axis is used to mark the orientation of the object being tracked, just check any OpenCV example that uses it:
https://docs.opencv.org/3.1.0/d5/dae/tutorial_aruco_detection.html

The axis is on the center of the tag, with the Z axis pointing outwards. What use is to have the axis lag behind (or overshoot) during movement potentially left being floating in empty space?

For the purpose of displaying the position from the driver it makes more sense to draw a small circle, which I do in 57499c2.

* [ ]  `Use hex numbering for tracker names in driver` - Shouldn't we just shorten the ApriltagTracker prefix for tracker names.

Probably would be better, yes.

* [ ]  `Fix missing null byte when sending IPC commands` - This isn't necessary from what I understand, but can be helpful if not using SOCK_SEQPACKET.

The driver then does this:

        if (ipcConnection.recv(buffer, sizeof(buffer)))
        {
            std::string rec = buffer;

And then if you read the definition for assiging a char * to a std::string from here: https://en.cppreference.com/w/cpp/string/basic_string/operator%3D
You'll notice it assumes the char * is a pointer to a null terminated string. So the above will crash unless you are lucky enough that the buffer just happens to be null terminated by pure chance (which I assume is the case on Windows).

It's just one byte. Just keep the null byte at the end for safety.

@funnbot
Copy link
Contributor

funnbot commented Jan 10, 2023

String fixes - could you explain what this does?

What build flags are you using? there are some utf8 handling changes that have a mess of build flags to attempt to fix the old way wxWidgets did this, for example the std::string constructor for wxString does not work with utf8. the U8String class I created is just a wrapper around std::string, and has a conversion operator that calls the correct utf8 conversion function before becoming a wxString.

  • Fix missing null byte when sending IPC commands - This isn't necessary from what I understand, but can be helpful if not using SOCK_SEQPACKET.

The driver IPC code shouldn't assume theres a null byte, but I agree adding it is fine for safety. Ill end up copying the IPC library in ATT over to the driver eventually.

@funnbot
Copy link
Contributor

funnbot commented Feb 5, 2023

Which changes should I prioritize so you can start using master?

@yoyobuae
Copy link
Contributor Author

yoyobuae commented Feb 7, 2023

Which changes should I prioritize so you can start using master?

Could start by fixing this crash that happens as soon as a tracker is detected while connected to SteamVR (this happens on the current master branch without any other changes):

Thread 12 "AprilTagTracker" received signal SIGTRAP, Trace/breakpoint trap.
[Switching to Thread 0x7ffff6a74700 (LWP 120997)]
0x00005555558e49e3 in Pose::Pose(cv::Point3_<double> const&, cv::Quat<double> const&)::{lambda()#1}::operator()() const (__closure=0x7ffff6a733f0) at /home/yoyobuae/Source/AprilTagTracker.3/AprilTagTrackers/Helpers.hpp:31
31              ATT_ASSERT(rotation.isNormal(), "pose rotation is a unit quaternion");
(gdb) bt
#0  0x00005555558e49e3 in Pose::Pose(cv::Point3_<double> const&, cv::Quat<double> const&)::{lambda()#1}::operator()() const (__closure=0x7ffff6a733f0) at /home/yoyobuae/Source/AprilTagTracker.3/AprilTagTrackers/Helpers.hpp:31
#1  0x00005555558e4b07 in Pose::Pose (this=0x7ffff6a734f0, pos=..., rot=...) at /home/yoyobuae/Source/AprilTagTracker.3/AprilTagTrackers/Helpers.hpp:31
#2  0x00005555558e74b0 in tracker::PlayspaceCalib::InvTransform (this=0x55555bcd5370, pose=...) at /home/yoyobuae/Source/AprilTagTracker.3/AprilTagTrackers/tracker/PlayspaceCalib.hpp:37
#3  0x00005555558e75ca in tracker::PlayspaceCalib::InvTransformFromOVR (this=0x55555bcd5370, pose=...) at /home/yoyobuae/Source/AprilTagTracker.3/AprilTagTrackers/tracker/PlayspaceCalib.hpp:49
#4  0x00005555558e9cd5 in tracker::MainLoopRunner::Update (this=0x7ffff6a73890, cameraFrame=..., gui=..., trackerUnits=..., vrClient=..., trackerCtrl=...) at /home/yoyobuae/Source/AprilTagTracker.3/AprilTagTrackers/tracker/MainLoopRunner.hpp:84
#5  0x00005555558e2b37 in Tracker::MainLoop (this=0x55555bcd5350) at /home/yoyobuae/Source/AprilTagTracker.3/AprilTagTrackers/Tracker.cpp:810
#6  0x0000555555910f46 in std::__invoke_impl<void, void (Tracker::*)(), Tracker*> (__f=@0x55555c228590: (void (Tracker::*)(Tracker * const)) 0x5555558e2998 <Tracker::MainLoop()>, __t=@0x55555c228588: 0x55555bcd5350)
    at /usr/include/c++/11/bits/invoke.h:74
#7  0x0000555555910e75 in std::__invoke<void (Tracker::*)(), Tracker*> (__fn=@0x55555c228590: (void (Tracker::*)(Tracker * const)) 0x5555558e2998 <Tracker::MainLoop()>) at /usr/include/c++/11/bits/invoke.h:96
#8  0x0000555555910dd5 in std::thread::_Invoker<std::tuple<void (Tracker::*)(), Tracker*> >::_M_invoke<0ul, 1ul> (this=0x55555c228588) at /usr/include/c++/11/bits/std_thread.h:253
#9  0x0000555555910d6a in std::thread::_Invoker<std::tuple<void (Tracker::*)(), Tracker*> >::operator() (this=0x55555c228588) at /usr/include/c++/11/bits/std_thread.h:260
#10 0x0000555555910d08 in std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (Tracker::*)(), Tracker*> > >::_M_run (this=0x55555c228580) at /usr/include/c++/11/bits/std_thread.h:211
#11 0x00007ffff78aa6b4 in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6
#12 0x00007ffff7b41609 in start_thread (arg=<optimized out>) at pthread_create.c:477
#13 0x00007ffff76fd133 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
(gdb) list
26      struct Pose
27      {
28          Pose(const cv::Point3d& pos, const cv::Quatd& rot)
29              : position(pos), rotation(rot)
30          {
31              ATT_ASSERT(rotation.isNormal(), "pose rotation is a unit quaternion");
32          }
33          explicit Pose(const RodrPose& pose);
34
35          /// When multiplied, applies no translation or rotation
(gdb) p pos
$1 = (const cv::Point3d &) @0x7ffff6a73440: {x = 0.27116073466332374, y = -0.023330620565180205, z = -0.71433317305440402}
(gdb) p rot
$2 = (const cv::Quatd &) @0x7ffff6a73460: {static CV_QUAT_EPS = <optimized out>, static CV_QUAT_CONVERT_THRESHOLD = <optimized out>, w = -0.22145834879372964, x = 0.020185498207277539, y = -0.76465578041010462, z = -0.4631812849836362}
(

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.

None yet

3 participants