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.

Sign up for GitHub

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

Jump to bottom

add check for nullptr handle in body track c++ shutdown() #1373

Closed
diablodale opened this issue Oct 4, 2020 · 4 comments
Closed

add check for nullptr handle in body track c++ shutdown() #1373

diablodale opened this issue Oct 4, 2020 · 4 comments
Assignees
@HlibKazakov2000
Labels
Body Tracking Issue related to the Body Tracking SDK Enhancement New feature or request Triage Approved The Issue has been approved by an Azure Kinect team member.

Comments

@diablodale
Copy link

The k4abt::shutdown() api doesn't check its m_handle before calling the C api k4abt_tracker_shutdown(). This is a minor inconsistency as compared to its sibling k4abt::shutdown() which does check its handle with if (m_handle != nullptr).

Ask

I request the handle test be added to k4abt::shutdown() so to be in alignment with its sibling shutdown() and remove boilerplate from user code.

Fix

Add the handle test to the C++ body track header like the following. While it is not necessary, I also added const to the function signature since it does not modify the class variables.

void shutdown() const noexcept
{
    if (m_handle != nullptr)
    {
        k4abt_tracker_shutdown(m_handle);
    }    
}

Alternatives

In a scenario of serial shutdown and destroy...

bodyTracker.shutdown();
bodyTracker.destroy();

User code can use a one line if test

if (bodyTracker) bodyTracker.shutdown();
bodyTracker.destroy();
@diablodale diablodale added Enhancement New feature or request Triage Needed The Issue still needs to be reviewed by Azure Kinect team members. labels Oct 4, 2020
@qm13 qm13 self-assigned this Oct 5, 2020
@qm13 qm13 added Body Tracking Issue related to the Body Tracking SDK Triage Approved The Issue has been approved by an Azure Kinect team member. and removed Triage Needed The Issue still needs to be reviewed by Azure Kinect team members. labels Oct 5, 2020
@qm13 qm13 assigned scotthsu98052 and unassigned qm13 Apr 13, 2021
@HlibKazakov2000
Copy link
Contributor

HlibKazakov2000 commented Apr 13, 2021 via email

@diablodale
Copy link
Author

Hi. I prefer to leave it open. I verify fixes for all the bugs that I open and aren't closed (against my wishes). I have so many open issues, that I must have tools (like open github issues) to manage them.

This allows somewhat the better phase changes of: open -> resolved -> closed.
But github is lacking that middle phase. So open is the only realistic alternative.

@qm13
Copy link
Collaborator

qm13 commented Mar 21, 2022 •
edited
Loading

Fixed in v1.1.1.

@qm13 qm13 closed this as completed Mar 21, 2022
@diablodale
Copy link
Author

@qm13, Fixed in what version? For customers to verify unseeable code changes, we need to know the exact version in which is was potentially fixed so that customers can verify, escalate non-fix, etc.

@diablodale diablodale mentioned this issue Aug 31, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@HlibKazakov2000 HlibKazakov2000

Labels
Body Tracking Issue related to the Body Tracking SDK Enhancement New feature or request Triage Approved The Issue has been approved by an Azure Kinect team member.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@diablodale @scotthsu98052 @qm13 @HlibKazakov2000