search

swri-robotics / descartes_light

Library for generation motion plans for process toolpaths
37 stars 21 forks source link

Feat/better logging #110

Closed marrts closed 4 months ago

marrts commented 11 months ago

Previously we had the << operator implemented, but there was no way to actually override it. Now you can override and implement the print method in any inherited class. Directly related to this PR in tesseract_planning.

Levi-Armstrong commented 7 months ago

What is the next step on this MR?

marrts commented 7 months ago

What is the next step on this MR?

From a Descartes perspective I don't have any other planned changes. @marip8 can give his input. On the tesseract side of things I want the referenced PR to be updated because the printout can get way too big. I want to make the trajectory collision object have summary methods which state the most common collision, worst collision, and maybe some other statistics. Then maybe something like that full detailed printout gets written to a file in /tmp somewhere.

Levi-Armstrong commented 4 months ago

Can this be merged?

Levi-Armstrong commented 4 months ago

@marrts Can this be merged?

marrts commented 4 months ago

@marrts Can this be merged?

This should be good to merge now.

marip8 commented 4 months ago

FWIW, this change requires a bump in minor version rather than patch version since we added a function that doesn't exist in previous versions. I'm okay force-pushing master to remove the last release and re-releasing with the correct minor version, but I think it will affect tesseract now that it has been updated to point to this version. @Levi-Armstrong how do you want to proceed? Maybe we can just make both updates at roughly the same time so it's not so much of a problem

Levi-Armstrong commented 4 months ago

Since it had a default implementation it should not break anyone's codes if they upgraded which is why I went with the patch update. I usually only increment the minor if breaking changes occur.

marip8 commented 4 months ago

From semver 2.0 summary:

MAJOR version when you make incompatible API changes MINOR version when you add functionality in a backward compatible manner PATCH version when you make backward compatible bug fixes

It should be a minor change since we added functionality (new method) in a backwards compatible way (default implementation)

Levi-Armstrong commented 4 months ago

See section 4 here, . This was previous discuss when defining Tesseract versioning. Since a 1.0.0 release has not happened then anything at any time can change, so to provide a little more structure when the major is zero we treat the minor version as if it was the major version when following semver and the patch version as if it was the minor version when following semver instead of just allowing anything to change.

Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable.