orocos-toolchain / orogen

Code generator for components and type handling in Rock - the Robot Construction Kit - and the Orocos Toolchain
http://rock-robotics.org
Other
4 stars 35 forks source link

fix return value of method_name in case typename contains a * #89

Closed jmachowinski closed 4 years ago

jmachowinski commented 7 years ago

This happens if for example Vector<Foo *> is given as opaque.

doudou commented 7 years ago

Maybe use something less prone to conflicts to replace the star with. __P__ ?

doudou commented 7 years ago

Actually, I definitely like the idea of replacing the markers by something that represents them (instead of by _ as done right now). Something along of the lines of the following should do the trick.

MARKER_TO_METHOD_STRING = Hash[
  '*' => "_P_",
  "[" => "_BROPEN_",
  "]" => "_BRCLOSE_",
  "," => "_COMMA_",
  "<" => "_LT_",
  ">" => "_GT_"]

def self.method_name
   ...
   gsub([\[\]<>*, ]) { |s| MARKER_TO_METHOD_NAME[s] }
end
doudou commented 7 years ago

If you could add tests at the end of test/gen/test_typekit.rb, that would be great as well (no need to do code generation, just check the name-to-method_name mapping)

jmachowinski commented 7 years ago

updated. For the tests, flexmock/minitest is not installed, and I don't have a clou how to install it these days... I suppose gem install is wrong now, as we use bundler ? With bundler I don't get how to install (bundle install gives me some errors, and googling does not help either)

doudou commented 7 years ago

Test dependencies are supposed to be installed when you enable tests for a particular package. However, orogen was missing these, so first apply https://github.com/orocos-toolchain/orogen/pull/90 and https://github.com/orocos-toolchain/autoproj/pull/29. Then

autoproj test enable orogen
autoproj osdeps

I'm converting the tests from Unit::Test to minitest/spec little by little, so please extend the describe block at the bottom of test/gen/test_typekit.rb like. And use the registry API to create new null types as test stubs, like so:

describe OroGen::Gen::RTT_CPP::Typekit do
   ...

   describe "Type.method_name" do
      before do
         @registry = Typelib::Registry.new
      end
      it "replaces pointer markers by a _P_ mark" do
          type = @registry.create_null '/std/vector</Pointer*>'
          assert_equal ...., type.method_name
      end
      it "replaces template <> delimiters by ..." do
      end
      .... and so on
   end
end

To actually run your tests, it's best for you to not run the whole file as quite a bit of the existing tests do run code generation and compilation (it's long ...). You can select tests with the -n /regex/ option:

ruby test/gen/test_typekit.rb -n /Type.method_name/
arneboe commented 6 years ago

Any updates on this PR? I am in the process of closing all repositories from project entern (as entern ended int 2017) and am following up on all PRs to see if they have been forgotten? :)

doudou commented 6 years ago

Well ... it wasn't as much forgotten as not followed through by the OP, plus since it was built on top of entern master, it got completely unrelated commits.

chhtz commented 6 years ago

I assume my workaround-commit is not necessary anymore -- but I haven't updated and built Entern for a long time. Replacing special chars with something verbose does indeed sound better. If you have time to do that (and add tests), go ahead, @arneboe :D

2maz commented 4 years ago

closed by #128