PerkLab / MCSTrack

Multi-camera spatial tracking
MIT License
0 stars 4 forks source link

Additional Information in Detector API #68

Closed vaughantnrc closed 3 months ago

keiranbarr commented 3 months ago

This looks good. However, I wonder how often we should be merging commits into main, because rebuilding the Pi OS images each time there is a merge seems a little excessive. Please let me know your thoughts.

vaughantnrc commented 3 months ago

Good point. I'm not currently planning any other API changes. That could change though. Let's wait a few days and see where we're at. I don't think either of the commits in this PR should affect the work that you or David are doing right at the moment (though for an accuracy evaluation we do want a correct detector frame timestamp).

keiranbarr commented 3 months ago

By my testing I think merging this PR would require an update of the Pi OS images. I get the following error when trying to connect to the detector (running the current main code) from the GUI (running this PR):

ERROR:src.common.status_message_source:source_label='controller' severity='error' message='Exception occurred in controller loop: ()' timestamp_utc_iso8601='2024-07-23T19:47:59.719988'

I can definitely update the OS images and merge this, it is not a problem. As you said, it is important to have a correct detector timestamp.

vaughantnrc commented 3 months ago

Yes, I meant to say that the change will not have any direct impact on the work you are doing, BUT you are right that it would need an updated Pi image. So I suggest let's wait a few days.

keiranbarr commented 3 months ago

Ah yes I see, thanks for the clarification. That all sounds good to me.

vaughantnrc commented 3 months ago

@keiranbarr I'm going to add requested resolution as per https://github.com/PerkLab/MCSTrack/issues/71

Then we can do the merge. Does that sound ok to you?