tesseract_planning icon indicating copy to clipboard operation
tesseract_planning copied to clipboard

Improve planning dictionary

Open Levi-Armstrong opened this issue 4 years ago • 3 comments

Currently the planning dictionary leverages strings and type_id which makes it easy to insert data under the wrong entry. In the past it was suggested to leverage boost strong typedef. I believe it would be good to update the profile dictionary to leverage this to prevent accidentally swapping the name and namespace. The type_id should be resolved with @marip8 upcoming refactor of the profiles.

#include <boost/serialization/strong_typedef.hpp>
#include <iostream>
namespace tesseract_planning
{
  BOOST_STRONG_TYPEDEF(std::string, ProfileNamespace)
  BOOST_STRONG_TYPEDEF(std::string, ProfileName)  
}

void test(const tesseract_planning::ProfileNamespace& ns, const tesseract_planning::ProfileName& name)
{
    std::string a = ns;
    std::string b = name;
    std::cout << "namespace: " << a << " name: " << b << std::endl;
}

int main()
{
    tesseract_planning::ProfileNamespace ns("profile_namespace");
    tesseract_planning::ProfileName name("profile_name");
    test(ns, name);
}

Levi-Armstrong avatar Nov 20 '21 17:11 Levi-Armstrong

It would also be good to use this for the working_frame and tcp_frame. @marip8, @mpowelson, @jdlangs any objections to replacing some of the strings with strong types to prevent user error?

  • profile namespace
  • profile name
  • working frame
  • tcp_frame

Levi-Armstrong avatar Nov 23 '21 15:11 Levi-Armstrong

I think strong types make sense for working frames and TCP frames because they both come from the same set of possible frames. It's easy and perfectly valid to accidentally swap the working and TCP frames, and you might not recognize (or even encounter) an error until much later, but the behavior is not what was intended.

I don't think it makes as much sense for the profile names and namespaces because those each come from their own set of possible strings. It seems unlikely that swapping the profile name and namespace will result in a valid configuration, and the existing checks should catch this. By forcing strong types for the names/namespaces, what used to be done in one line now takes three, and it doesn't seem like it adds that much value

marip8 avatar Nov 23 '21 15:11 marip8

For what it's worth, I tried adding this to some changes I am making to the ProfileDictionary, and things don't compile because the strong typedefs are not serializable by boost

marip8 avatar Oct 10 '22 21:10 marip8