karnikram / autocalib-sensor-extrinsics

2018 GSoC Project with MRPT
BSD 3-Clause "New" or "Revised" License
12 stars 6 forks source link

m_sync_obs_indices highly confusing #16

Closed EduFdez closed 6 years ago

EduFdez commented 6 years ago

Hi @karnikram There are many issues with m_sync_obs_indices that reveal bad programming practices: 1) It's declared in the wrong place. Since it is a variable related to the dataset it should not be in the GUI. The best class in the project where it could be is ExtrinsicCalib, though I think it's better to create a new class in core/utils to deal with synchronization, and other dataset related issues like the list of sensors to calibrate, the maximum offset (you call it delay) for "syncronized" obs, etc. 2) You declare m_sync_obs_indices twice in CMainWindow and CCalibFromPlanesGui for a same purpose. This does not make sense. Would you create a new m_sync_obs_indices in CCalibFromLinesGui?