--------------------------------------------------------------------------------
TODO - velodyne-genom
--------------------------------------------------------------------------------

Categories:
- Build
- Code
- Documentation
- Formatting
- General
- Interface
- Naming

Sections:
- To be verified - Bugs
- To be verified - Improvements
- To be verified - Features
- Confirmed - Bugs
- Confirmed - Improvements
- Confirmed - Features

--------------------------------------------------------------------------------
To be verified - Bugs

Build:
- velodyne3DImage.h and velodyneTarget.h: should we instantiate these files in
  the makefiles?

Code:
- Playback mode random shots: apparently the number of shots changes each time
  the playback mode is launched, which is unexpected and could be considered a
  bug.
- pthread_exit: there's no call to this function, even though we recreate the
  read thread at each call to StartAcquisition.
- OneShot and Read synchronization revision: shouldn't OneShot unlock the mutex
  before signaling the condition? Check 'OneShot error handling'.
- Reading and POM synchronization counter overflow: a counter, 'i', is used to
  synchronize the reading with POM, and there's no check regarding its
  overflow. Theoretically, if the program runs for an indefinite amount of time,
  this counter will reach its limit and it's thus necessary to do this check.

--------------------------------------------------------------------------------
To be verified - Improvements

Build:
- sys/time.h inclusion in codels: check wether it's necessary.

Code:
- Bool type: review its use throughout the module. Example: the variable
  'running' is a boolean, but is defined as int because it must be parsed by
  Genom. The use of bools should be consistent throught the module. How are
  conversions between int and bool handled? Are there other similar cases?
- SaveIm3D request: check wether we're saving as much information as possible.
- Variable 'running': shouldn't it be incorporated to velodyneReadTask?

Formatting:
- switch or if-else-if convention. Which one should be used, and in which case?
  It would be nice to come up with a convention and apply it consistently
  throughtou the module. Example: blocks in velodyneInitMain.

Interface:
- StartAcquisition and StopAcquisition: shouldn't these requests be symmetric,
  that is, shouldn't both be control codels, or exec codels? Otherwise, the
  names should change, or else there could be a velodyneAcquisitionStartCntrl,
  velodyneAcquisitionMain and velodyneAcquisitionStopCntrl, or something
  similar.

--------------------------------------------------------------------------------
To be verified - Features

Code:
- Sensor-to-main transformation: is it possible to move the computation of
  Velodyne heading offset from OneShot to Init? More generally, how should we
  deal with the transformation, is it dynamic or not?

General:
- OneShot sector: is it worth creating a structure that adapts itself to the
  discarded sector of the scan, in order to ease the task of client modules?
- OneShot sector approximation: study the impact of this approximation, caused
  by the organization mechanism. Should we deal with this in the code, or maybe
  by improving the fov filter in velodyne-libs?
- Module design: a complete restructuration of the module is desirable. The
  ideal would be to create a separate library, leaving only wrapping codels in
  the module. Is this possible? Check Confirmed - Features. Some problems:
  - 3D image, scan or shot: these three concepts are linked together, but are
    not clearly defined and are somehow confusing. We have the request OneShot,
    the internal structure 3DImage, the external structure DATA_IM3D, the action
    of scanning, the function grabOneShot in the file grabOneScan, the file
    velodyneClient.h. These concepts could be better defined and files and names
    changed accordingly.
  - grabOneShot.c: check the separation between the functions in the file.
  - Data structures redefinition: is there a way to avoid the redefinition of
    structs provided by velodyne-libs? Are there structures we can reuse? In the
    case of the 3D image, the ideal would be to reuse an external structure for
    the 3D images, or else implement it in an external library or separate
    section.
  - Concepts paradigm: define precisely the concepts involved in the code, and
    update comments accordingly. Example of concepts: reference frames (robot,
    sensor, original Velodyne), heading/angular/horizontal-fov/yaw, etc.

Interface:
- Modules synchronization: the module should be robust with respect to the state
  of other modules, and in the ideal case, independent. Example: the module
  should wait for POM, in case it wasn't started before. Is this functionality
  already implemented by the supervisor? Also, in
  velodynePOM_SENSOR_POSPosterRead, the call to this function in codel
  velodyneReadMain depends on a hard-coded value representing the POM frequency.
  This should be changed. Moreover, the currently fixed frequency must also be
  reviewed, it could be that it's not the most appropriate frequency anymore.

--------------------------------------------------------------------------------
Confirmed - Bugs

General:
- Organization: review the system for both sensors, because the current
  implementation is based on a false model. Check the consequences of keeping
  the current model. Think about calculating the vertical alignment of columns
  in a proper way, using the mean number of packets received by the models or
  computing it online in an adaptive manner. Related to velodyne-libs.

Interface:
- Return errors: the return possibilities of many functions don't correspond to
  the ones indicated in comments. Review the whole error and report system.
  Examples: velodynePOM_SENSOR_POSPosterRead, velodyneSaveMain and
  velodyneSaveFull3DImageMain.
- Binary file output: the images3d binary file format, as well as the internal
  binary file format, are not portable across different architectures and their
  use should thus be reviewed. Old datasets cannot be properly used due to this
  problem.

--------------------------------------------------------------------------------
Confirmed - Improvements

Code:
- const function parameters. const should be added where appropriate. Example:
  functions of target detection and tracking.
- Make ExecTaskEnd symmetrical to ExecTaskStart. If necessary, another request
  or codel should be introduced.
- Save functions: implement them in file velodyneReader.c.
- grabOneShot constants: replace values like 11 and 36000 by defined constants.
- velodyneRead position read: make the position reading symmetric to the packet
  reading, with respect to the code, that is, using directly the buffer's
  address.

Documentation:
- English documentation: translate all the comments to English. Example: SDI ->
  IDS.
- .gen file: document the .gen file, adding comments about the module and
  explaining each of the requests and its arguments. Update doc fields of the
  requests. Synchronize the gen file documentation with the codels
  documentation.

Formatting:
- Parentheses: there are some unnecessary parentheses, for example in if
  conditionals and in += expressions and returns.

Interface:
- VELODYNE_FILE_FORMAT: move variable out of velodyneStruct.h.
- Error messages format: decide on a convention and apply it.
- Informative default values removal: if a parameter is mandatory and must be
  set, then there shouldn't be a default value for it.
- Improve reading error-handling. Check for possible errors, introduce error
  codes in returned values.
- Error handling comparisons: implement the two-steps error handling strategy
  throughout the module, in order to make the code faster when there's no error.
- OneShot error handling. Check 'OneShot and Read synchronization revision'.
  Check grabOneShot results. Improve the error results and define them in the
  header.

Naming:
- Formatting: adopt upper case initial letters for enums and structs.
- velodyneMode: this name is not consistent with the velodyne-libs nomenclature.

--------------------------------------------------------------------------------
Confirmed - Features

Build:
- Introduce testing.

Code:
- Reading of pcap files beginning with incomplete packets.

General:
- Separate scanning from target tracking: separate the target tracking part from
  the module. Perhaps it would be better to start by making a branch. Target
  detection module:
  - Move as much target detection functionality as possible from hyper to the
    module. Ideally, hyper should only be responsible for the supervision, not
    for the detection process itself.
- Reset functionality: create an End request, or maybe a Reset request, or come
  up with a way of restarting the module and testing different parameters
  without having to kill it first.
- File versioning.
- Frame convention: the convention should be to work in the modified Velodyne
  frame, not in the original one. Related to velodyne-libs.
- Create reader module: move all reading functionality into a reader module.
  Clean codels, create separate functions, split Genom functionalities from
  reading functionalities. Check To be verified - Features.
- Create a module status request or poster.

Interface:
- velodyne3DImage: make the structure and poster similar to the one in the
  stereo-pixel module, even equal if possible.

--------------------------------------------------------------------------------
