ivapylibs / Surveillance

The surveillance system for the SuperviseIt project
0 stars 1 forks source link

No hand coding | Use configuration files #14

Open pv33 opened 2 years ago

pv33 commented 2 years ago

The idea of hand-coding execution is horrendous. It's probably one of the worst things one can do from a collaborative coding perspective. It makes consistent and predictable implementations infeasible. It also prevents others from reliably using the code for their needs when there is overlapped use of the same code files.

The proper way to do this is through configuration files, either as distinct scripts (bad idea but sometimes tolerable) or as editable configuration files with a master file that never gets touched to avoid undermining versioning in the repository.

I've gone ahead and kludged in loading of configurations from yaml specifications. Please make sure that you follow this paradigm when moving forward. I am not sure how you are programming things, but my objective is for code to look native to the command line. My revisions therefore exist in two new command line executable functions: rbreplay and rbrun that reproduce the rosbag_replay.py and rosbag_runner.py functionality but snag the settings from a yaml file.

Execution (when in the ROS directory) would be:

./rbreplay config/xxPuzzleSolver01.yaml

or

./rbrun config/xxPuzzleSolver01.yaml

Note that the above rbrun actually crashes because something is missing. I forget what it is, but you'll see it crash (for the yiye branch). There is another version with different flag settings to just visualize the hand tracking. It would be:

./rbrun config/xxHandTracking01.yaml

This one runs without problems.

Of course the real files have test as the prefix, but you should NOT edit those files. Instead copy the files to be such that xx are your initials. Do NOT add those copied files to the repository. They'll exist for you and you only. You should only be changing the non-critical parameters (source, general,output) but not any of the critical run-time parameters. Doing so establishes reproducibility by everyone under the presumption that we are loading the same files.

If you have very specific test cases that should be added, then you should be adding them as testCASENAME##.yaml but following the original field information in the functional test case yaml files. I'll be checking them for correctness. Your own copy should be what you use. An alternative if you really want to push your versions to the repository for being able to work on multiple machines is to create your own sub-directory in the config directory. example config/pav/testCASENAME##.yaml. In that case, the name can be the exact same. Each person can then run their own version and there won't be conflicts. The base config folder will have the original version for which specific settings are frozen. It might even be best to specify which should be frozen through a comment like:

field:  value       # do not change

or by specifying which are editable

field: value        # editable

YAML files permit hash comments.

Notes

It might be best to have a source:path yaml specification and leave the rosbag and puzzle parts to have only the filename. If you want to trace the source and make that modification, then you can do so. I don't see it as mission critical and am happy to leave alone. In fact, it just add to work without fixing things. Eventually, there will be a better way to do things anyhow.

The above holds for the yiye branch. Changes will need to be merged if you are using a different branch.

@yiyeChen @Uio96

yiyeChen commented 2 years ago

@pv33 @Uio96

I would like to recommend a tool called yacs to handle the yaml configuration files. The yacs.config.CfgNode class offers numerous handy functions related to the configuration management, include loading from the yaml file, merging different set of configurations, and freezing the parameters. Some example are provided in their README.md in the link above.

Also it is easy to add new features from their base class. For example I have added the loading function from a list of yaml files (the corresponding commit: d570e293524379d1956500b1be0b7281d43a8c6f). This allows us to split the parameters for different system (e.g. Surveillance, puzzle solver) into separate yaml files. This provides flexibility since we might want to add more parameters during the code development. By splitting them as opposed to create multiple copies containing a great chunk of shared parameters, we don't have to modify all the previous yaml files.

I have already modify the rosbag_data_recorder.py to read the yaml files, since Juseob will need a convenient way to modify the camera parameters. Will do that for the others when necessary.