Skip to content

Conversation

Copy link
Member

@brosenberg42 brosenberg42 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @jrobble)


detection/api/include/MPFDetectionComponent.h line 248 at r2 (raw file):

        virtual std::vector<MPFVideoTrack> GetDetections(const MPFVideoJob &job) = 0;

        virtual std::vector<MPFVideoTrack> GetDetections(const MPFAllVideoTracksJob &job) {

I think this could cause confusion since a C++ developer could override this method. We could remove this method and throw the exception in CppComponentHandle. Then, C++ component developers would not see the method at all.

Copy link
Member Author

@jrobble jrobble left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @brosenberg42 and @jrobble)


detection/api/include/MPFDetectionComponent.h line 248 at r2 (raw file):

Previously, brosenberg42 wrote…

I think this could cause confusion since a C++ developer could override this method. We could remove this method and throw the exception in CppComponentHandle. Then, C++ component developers would not see the method at all.

Removing this method results in this error:

/home/mpf/git/openmpf-projects/openmpf/trunk/detection/executor/cpp/cli-runner/MpfCppSdkPythonBindings.cpp:189:100: error: no matching function for call to ‘MPF::COMPONENT::MPFDetectionComponent::GetDetections(const MPF::COMPONENT::MPFAllVideoTracksJob&)’
  189 |                  [](CppComponent& c, const MPFAllVideoTracksJob &job) { return c->GetDetections(job); },
      |

I had to also update MpfCppSdkPythonBindings.cpp:

            .def("GetDetections",
                    [](CppComponent& c, const MPFAllVideoTracksJob &job) {
                        throw std::runtime_error{"MPFAllVideoTracksJob is not currently supported."}; 
                    },
                    "job"_a,
                    call_guard_t())

Copy link
Member

@brosenberg42 brosenberg42 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @jrobble)


detection/api/include/MPFDetectionComponent.h line 248 at r2 (raw file):

Previously, jrobble (Jeff Robble) wrote…

Removing this method results in this error:

/home/mpf/git/openmpf-projects/openmpf/trunk/detection/executor/cpp/cli-runner/MpfCppSdkPythonBindings.cpp:189:100: error: no matching function for call to ‘MPF::COMPONENT::MPFDetectionComponent::GetDetections(const MPF::COMPONENT::MPFAllVideoTracksJob&)’
  189 |                  [](CppComponent& c, const MPFAllVideoTracksJob &job) { return c->GetDetections(job); },
      |

I had to also update MpfCppSdkPythonBindings.cpp:

            .def("GetDetections",
                    [](CppComponent& c, const MPFAllVideoTracksJob &job) {
                        throw std::runtime_error{"MPFAllVideoTracksJob is not currently supported."}; 
                    },
                    "job"_a,
                    call_guard_t())

Now that you have updated MpfCppSdkPythonBindings, can this be removed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants