Port to C++14 about mrpt HOT 68 CLOSED

I think this one is covered right?
Use modern CMake 3.1+ C++ compiler features: target_compile_features, WriteCompilerDetectionHeader, set (CMAKE_CXX_STANDARD 11), etc.

from mrpt.

I don't think it would be useful to check that every class to be movable universally. The PDF variants of the poses definitely have the most to gain from move semantics and I think that is already covered. That particular item can be checked off the list.

from mrpt.

Done with "Add constexpr to geometry constructors, etc."

from mrpt.

Just deleted this one from the list, since it didn't have actually anything to do with C++14.

• Create Qt-like macros to easily declare simple member variables + getter + setter, eg. DECLARE_PROPERTY(double, Width, m_width ) --> double m_width; double getWidth() const;, ...

Also, I'm unsure about it usefulness right now...

from mrpt.

"Important: Before GCC 5.1 support for C++11 was experimental. Some features were implemented based on early proposals, and no attempt was made to maintain backward compatibility when they were updated to match the final C++11 standard." - https://gcc.gnu.org/projects/cxx0x.html

I use c++11 features on Ubuntu 14.04(gcc 4.8.4) and it is usable.

It's worth noting that gcc 5.1 is the compiler used in Ubuntu 15.10. Ubuntu 16.04 LTS will probably use 5.3. Unfortunately, gcc 6 will probably not be ready before the 16.04 LTS, which will have gnu++14 as the default c++ dialect. I believe gcc 5.1 still defaults to gnu++98, so you'll have to add -std=gnu++11.

https://gcc.gnu.org/develop.html#timeline

from mrpt.

Thanks for the info!

I think that the limiting factor seems to be gcc in Ubuntu precise 12.04 (which we support in our PPA), which is supported until 2017/04 and comes with gcc 4.6.3.
Still, a usable subset of C++11 features was ready for gcc 4.6, so the porting may be possible this year without dropping support for precise.

from mrpt.

CPipe uses std::auto_ptr which is deprecated in c++11. Not only is the warning annoying, but std::auto_ptr will be removed in c++17. Version 2.0.0 would probably be a good time to change that interface.

It might work to template that method. It should maintain compatibility to older code.

typedef std::shared_ptr<CPipeWriteEndPoint> CPipeWriteEndPointPtr;
typedef std::shared_ptr<CPipeReadEndPoint> CPipeReadEndPointPtr;

template <typename ReadPtr, typename WritePtr>
static void createPipe(ReadPtr &outReadPipe, WritePtr & outWritePipe)
{

from mrpt.

Thanks for the idea!
Sure, it'll be done for 2.0.0.

Also, you may have noticed that as a workaround to avoid those warnings I opened #235 , because CPipe is actually used in only a couple of places in the entire mrpt...

from mrpt.

@jlblancoc probably want to add "support move semantics"

Sometimes you get move implicitly. Here are the rules.

Implicitly-declared move constructor
If no user-defined move constructors are provided for a class type (struct, class, or union), and all of the following is true:
there are no user-declared copy constructors;
there are no user-declared copy assignment operators;
there are no user-declared move assignment operators;
there are no user-declared destructors;

Eigen should support move already, but I don't think any mrpt classes have an explicit move constructors. It's very easy to break one of those above rules without realizing it. Probably the best place to start is to see if everything derived from CArrayNumeric supports move.

from mrpt.

Thanks! I added that to the list in the issue text above. Indeed, that should be done to allow users exploit move semantics.

If you have an idea of how to perform a generic compile-time or runtime test that fails if the "move" op. does not happen, we would add it to be done as part of the unit tests, just like it's done now with serialization ;-)

from mrpt.

@jlblancoc

It's already in the standard.

This should give you good error messages:

#include <type_traits>

struct CannotMove
{
    CannotMove(const CannotMove &)
    {}
    CannotMove(CannotMove &&) = delete;
};

struct CannotCopy
{
    CannotCopy(CannotCopy &) = delete;
    CannotCopy(CannotCopy &&){};
};


template<typename T>
class ConstructorTests{
  static_assert(std::is_move_constructible<T>(), "Can't move");
  static_assert(std::is_copy_constructible<T>(), "Can't copy");
};

template class ConstructorTests<CannotMove>;
template class ConstructorTests<int>;
template class ConstructorTests<CannotCopy>;

gcc

example.cpp: In instantiation of 'class ConstructorTests<CannotMove>':
23 : required from here
19 : error: static assertion failed: Can't move
static_assert(std::is_move_constructible<T>(), "Can't move");
^
example.cpp: In instantiation of 'class ConstructorTests<CannotCopy>':
25 : required from here
20 : error: static assertion failed: Can't copy
static_assert(std::is_copy_constructible<T>(), "Can't copy");
^
Compilation failed

clang

19 : error: static_assert failed "Can't move"
static_assert(std::is_move_constructible<T>(), "Can't move");
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
23 : note: in instantiation of template class 'ConstructorTests<CannotMove>' requested here
template class ConstructorTests<CannotMove>;
^
20 : error: static_assert failed "Can't copy"
static_assert(std::is_copy_constructible<T>(), "Can't copy");
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
25 : note: in instantiation of template class 'ConstructorTests<CannotCopy>' requested here
template class ConstructorTests<CannotCopy>;
^
2 errors generated.
Compilation failed

Might want to add
is_move_assignable
is_copy_assignable

Also this might be useful for some specific tests.

template <class T, class U> struct is_assignable;

from mrpt.

Brilliant!

BTW: I just created a 2.0-devel branch to slowly start pushing changes there.

from mrpt.

@jlblancoc It does look like there are some issues with move as shown by my pull request.
https://travis-ci.org/MRPT/mrpt/builds/134300599

/home/travis/build/MRPT/mrpt/samples/traits_test/test.cpp:36:3: error: 
      static_assert failed "Can't move"
  static_assert(std::is_move_constructible<T>(), "Can't move");
  ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/travis/build/MRPT/mrpt/samples/traits_test/test.cpp:43:16: note: in
      instantiation of template class 'TraitsTest<mrpt::poses::CPoint2DPDF>'
      requested here
template class TraitsTest<mrpt::poses::CPoint2DPDF>;
               ^
/home/travis/build/MRPT/mrpt/samples/traits_test/test.cpp:36:3: error: 
      static_assert failed "Can't move"
  static_assert(std::is_move_constructible<T>(), "Can't move");
  ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/travis/build/MRPT/mrpt/samples/traits_test/test.cpp:46:16: note: in
      instantiation of template class 'TraitsTest<mrpt::poses::CPointPDF>'
      requested here
template class TraitsTest<mrpt::poses::CPointPDF>;
               ^
/home/travis/build/MRPT/mrpt/samples/traits_test/test.cpp:36:3: error: 
      static_assert failed "Can't move"
  static_assert(std::is_move_constructible<T>(), "Can't move");
  ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/travis/build/MRPT/mrpt/samples/traits_test/test.cpp:54:16: note: in
      instantiation of template class 'TraitsTest<mrpt::poses::CPose3DPDF>'
      requested here
template class TraitsTest<mrpt::poses::CPose3DPDF>;

from mrpt.

@jlblancoc: Might as well add c++17 to the list since they will be adding parallel policies. http://en.cppreference.com/w/cpp/algorithm

I think those would be preferred over Intel TBB based solutions.

from mrpt.

How can I join this initiative?

from mrpt.

@olzhas Take a look at the 2.0 branch.
It's fairly stale right now. You'll want to merge master into 2.0. That would be a great first step.
Once you're done, fork it, send a pull request. Make sure it's to the right branch(your 2.0->mrpt 2.0).

I would really like to see stlplus shared_ptr replaced with std::shared_ptr.

Let me know if you need any help.

from mrpt.

from mrpt.

Agreed, I think supporting const will be a great second step. There's quite a lot of infrastructure with CObject which may make binding pointers smart pointers of different types a bit tricky, so I didn't want to overwhelm him. I think just the conversion to std::shared_ptr will be a good start.

I just didn't want to scare @olzhas away. Stripping out stdlib pointers is a considerable undertaking, so I wanted to limit the scope.

RE: CObject
I was thinking...there are some tricky things that happen with derived types, CObjects and pattern matching used for observations(i.e. IS_CLASS(obs, CObservation2DRangeScan)), which could be refactored into std::variant. It makes me wonder if the use of CObject adds undue complexity to each class. Perhaps delegating the responsibility that CObject currently has to several decoupled classes might make things simpler. Of course variants are c++17 feature(http://en.cppreference.com/w/cpp/utility/variant), so I was wondering if we should re-target the 2.0 milestone for c++17 prior to going too deep down the rabbit hole. Of course, removing CObject would certainly be a massive undertaking.

I think it's cleaner to have each class typedef their own smart pointer, but I think it would break a lot of what CObject currently does. For example:

class MyClass
{
      public:
        typedef std::shared_ptr<MyClass> Ptr;
        typedef std::shared_ptr<const MyClass> ConstPtr;
};

from mrpt.

Didn't know std:: variant, cool! Will see it's supported compilers,
though...

Just a note on your last proposal for typedefs: I'm on the phone now and
can't see it, but I'm almost sure that there is already a typedef named Ptr
inside each CObject, in one of the DEFINE_OBJECT* macros...
Only, that we also have the other macros outside of each class which are
redundant and define the FooPtr names, much more common since they were
used from the beginning of MRPT. But for simplicity, if this allows
dropping some of the required macros before and after each class
declaration, we could drop the FooPtr names and leave only Foo::Ptr.

We'll see...

from mrpt.

I think variants are a late add to the c++17 standard. Work in gcc only started a little over a month ago.
It's similar to the boost variant though.
https://github.com/gcc-mirror/gcc/commits/master/libstdc%2B%2B-v3/include/std/variant

I have to admit that I still struggle with those DEFINE_OBJECT macros. I find them very confusing.

The nice thing about std::variants is that it that a single collection of variants can store very disparate types without requiring inheritance. It which helps preserve the sanity of your inheritance structure.

Also, the visitor pattern is almost like pattern matching from haskell.
http://en.cppreference.com/w/cpp/utility/variant/visit

from mrpt.

@jolting got it.

from mrpt.

@olzhas This is what I had in mind with the variant:
jolting@0dbaa3c
For now the boost::variant will do, but I want to replace it with std::variant. I definitely don't have it all working yet, but I think it should make the macros unnecessary. It's not in any buildable state, so that's why it's on my work in progress branch.

I still need to tackle the type info thing and refactoring a couple of places where CLASS_ID is checked which could be simplified using templates(i.e. CPoseRandomSampler).

Take a look at CSerializable.h that uses the boost::variant for now. It should be trivial to replace this with std::variant.

Notable take a look at libs/base/include/mrpt/utils/CStream.h. It has a templated operator >> and operator << which should make conversion to and from CSerializableOption. boost::get should throw an exception if the types don't match, which can easily be caught and rethrown into a reasonable exception message.

Now each class just has a readStream, writeStream which can be called with a templated method for each observation type instead of using polymorphism, which seems like it's overkill for an interface.

from mrpt.

Hi @jolting !

wow, a lot of critical changes there :-)
One side that I like of your proposal is the drastic reduction of LOCs, of course.

But, on the other side, the std::variant idea is the only one in all "modern C++XX" for which I feel somewhat reticent. Let me motivate it:

• Scalability / maintanability: The list in jolting@0dbaa3c#diff-747ea90dd2801dd93b1c69954f1c4c0bR29 seems a weak point of the approach, doesn't it?? Right now, one can define new CSerializable objects in 3rd party libs (e.g. a private lib linked against mrpt) and the CSerializable mechanisms will work seamlessly with all the classes, inside and outside MRPT's source tree.
• How could one address the problem of deserialization without knowing the type in advance (e.g. a rawlog) with this? This comes down to the issue of having a class factory that, given a class name as a string, creates a new object of that type.
• The cost for Windows users: one feature I've struggled (that's the word!) to maintain over the years is keeping the dependency hell under control for Windows. If std::variant is not supported in MSVC2015... we enforce users to use Boost, which is a monster on itself... and which should be distributed in binary form for precompiled versions of MRPT.... doable, but until now I kept apart of it as much as I could ;-)

What do you think??

from mrpt.

Dependency Hell

I think the variant type doesn't suit the problem exactly, but I think it's a step in the right direction. Perhaps the solution is to create our own type by leveraging some C++11 features.
I certainly don't want a final solution to depend on boost. It's just a means to an end, so I definitely want something that works easily on all platforms. Given that c++17 isn't available yet, I was thinking about creating an MRPT class that implements as much of the functionality with only c++11 features. Boost just had something that partially worked, so I thought I'd start from there.

Deserialization

In c++11 we can use type indexes and create a hash lookup for the type name.
Example: http://en.cppreference.com/w/cpp/types/type_index
This should provide the ability to go from type name to type fairly easily.

Scalability / maintanability

It seems that there is already partially a problem with this.
https://github.com/MRPT/mrpt/blob/c723c8fe02b96d3ae40dd5a8094762cd7ed0a588/libs/base/src/registerAllClasses.cpp

Actually what we really need is std::any(forgive me for throwing another c++17 feature in there)
http://en.cppreference.com/w/cpp/utility/any
I think we can probably implement something that does that partially in c++11 that will use the type_index and work for any type.

from mrpt.

Forgive the prototype nature of the code, but Perhaps something like this:
jolting@653021a
Solves Scalability and Portability.

from mrpt.

Any is much, much better, thanks!! 👍

I may have lost something, but I think that the only missing critical feature of this approach (without the ugly old macros) is the lack of a class factory, required for reading from rawlogs, etc. right?

Do you think it's possible to do it somehow without relying on a central class registry, etc. etc. as it's done right now?

from mrpt.

I think so, but I'll have to mull it over before trying some more brutal refactoring tricks.

I believe the context in which the rawlog is read really decides what types observations really need to be in the registry. You shouldn't need code to de-serialize an object you never intent to use(I think). Maybe the registry can just be local for the types that you want to read.

RawLogReader<OBS1,OBS2,...OBSn> reader;

from mrpt.

Just to keep this thread alive: during these months I've been updating the checklist at the top with related tasks. I hope we soon will have time to finally release 1.5.0 and put hands on 2.0.0!

The only change I'm still reluctant to undertake is a replacement based on Any above... It's more elegant, agreed, but I can't see how to maintain the flexibility of registering classes from different DLL's/.so's dynamically, etc. The current system is a bit ugly with all those macros, but is really well tested... and works! :-) We'll have to think much more on this particular item.

from mrpt.

stlplus task strike out after PR #461 ... 👍

from mrpt.

"Prefer nullptr" strike out after PR #467

from mrpt.

Make all destructors noexcept

The default is noexcept if unspecified.

"non-throwing functions are all others (those with noexcept specifier whose expression evaluates to true as well as destructors, defaulted special member functions, and deallocation functions)"

from mrpt.

The default is noexcept if unspecified.

Oh! That's great then... thanks. Marking the item as "done"... ;-)

from mrpt.

What do you think about a new c++14 simple serialization scheme?

It's kinda based on the following idea:

#include <tuple>
#include <iostream>
#include <utility>
#include <iostream>

class Serializer
{
        template <typename T>
        void apply_impl(T &a, std::index_sequence<>)
        {
//base case
        }

        template <typename T, std::size_t value, std::size_t ...values>
        void apply_impl(T &a, std::index_sequence<value, values...>)
        {
                std::cout << std::get<value>(a) << std::endl;
                apply_impl(a,std::index_sequence<values...>{});
        }

        template<typename T, typename I = std::make_index_sequence<std::tuple_size<T>::value>>
        void apply(T &a)
        {
                apply_impl(a,I{});
        }

 public:
        template <typename T>
        void  serialize(T& t)
        {
                apply(t.annotation);
        }
};

class MyClass
{

  public:

  int a = 0;
  int b = 1;
  int c = 0;
  std::string str = "foo";
  std::tuple<int,int,int, std::string> annotation = {a,b,c,str};
};


int main()
{
   Serializer serializer;
   MyClass foo;
   serializer.serialize(foo);
}

std::tuple won't hold references, so a custom tuple-like wrapper would be needed.

from mrpt.

I understood it (which is something! haha ;-). Cool design! 👍

Obviously this is more beautiful and elegant than the current monster-spaghetti code in all virtual read/write methods. I see one important downside, though: how could we handle versioning? Perhaps with a list of annotations, one per version?

I don't discard this solution if we manage to find a reasonable solution to this point... Eventually we might find some game stopper, but in principle... it looks doable!

Anyhow, being able to read old datasets (e.g. from 2, 3, 5 or 9 years ago!!) is an irrevocable design goal: there are many MRPT datasets around, not only internally to our research labs, but in public dataset repositories, etc.

PS: On C++14, my thoughts were to enforece C++14 for MRPT 2.0, not only C++11. I think GCC/CLANG/MSVC are updated enough for this.

from mrpt.

PS: On C++14, my thoughts were to enforece C++14 for MRPT 2.0, not only C++11. I think GCC/CLANG/MSVC are updated enough for this.

The std meta-programming functions are c++14, but mostly c++11 will support them if a rewrite is necessary.

Anyhow, being able to read old datasets (e.g. from 2, 3, 5 or 9 years ago!!) is an irrevocable design goal: there are many MRPT datasets around, not only internally to our research labs, but in public dataset repositories, etc.

I thought about a list per version, but that might be very long. This might be more directly translatable.

Annotation<
 Annotate<int32_t, 0, 9>,
 Annotate<int32_t,1,3>,
 Annotate<int32_t,3,4>,
 Annotate<std::string,3,9> annotation = 
 { a, b, c, d};

This assumes the first number is the first version and the last number is the version the variable was removed.

I was also thinking of using the Annotation type information to create ROS msg files. Unfortunately passing type names is a bit tricky to do with templates because string literals aren't directly supported. It's not impossible though. Autogenerated ROS MRPT bridge code would be pretty cool.

from mrpt.

@jolting Hunter, please note my recent commit e1f9b26 .

I am concerned with maintaining existing MRPT codebase, in particular, ROS repositories, which must be compiled against older versions of MRPT (from different Ubuntu versions), so I added that "transitional" feature of having using CFooPtr = CFoo::Ptr;.
I decided to switch it on by default, at least for MRPT 2.0. It could be set to off by default in MRPT 2.1 and removed at some time in the future.

from mrpt.

Added item to the checklist on mrpt::utils::delete_safe.

from mrpt.

Two more items marked as done (threads; semaphores & mutexes) after #488

from mrpt.

Review usages of mrpt::utils::delete_safe and consider changing them to use C++11 smart pointers.

I think all explicit new and deletes should be removed from mrpt unless there are really good reasons not to do so. Instead all dynamic memory allocation should be either done with std::make_shared or std::make_unique.

@olzhas What do you think?

from mrpt.

@jolting I agree with you, explicit new and delete must be removed. What if we choose std::shared_ptr for all the cases and pass them (maybe by reference), even though it might add additional overhead compared to std::unique_ptr?

from mrpt.

Instead all dynamic memory allocation should be either done with std::make_shared or std::make_unique.

Just FYI / for fun: Only for particularly-computational intense functions, a good optimization is to use stack dynamic allocation with alloca(). There is a wrapper mrpt_alloca() that falls back to malloc() in systems without alloca(). It's well suited to places where a temporary point-cloud or list of small objects needs to be created with a size unknown at build-time; naturally, we're limited to the stack limit, so this does not apply to huge memory blocks.
Right now, this trick is only used in a couple of places in MRPT.

In fact, any optimization change should have be measured, since the system dyn. allocator may be not as bad as we assume it is by default...

from mrpt.

We need an mrpt::AllocaAllocator
Then...

float	*scanPoints_x = (float*) mrpt_alloca( sizeof(float) * nRanges );

Should be...

std::vector<float,mrpt:::AllocaAllocator> scanPoints_x(nRanges);

from mrpt.

from mrpt.

What about something like an in-process nodelet like messaging service?

I think it could depend on any, which right now is only available in the experimental stl library.

Threads could be added, but here's a prototype:

#include <memory>
#include <unordered_map>
#include <mutex>
#include <list>
#include <experimental/any>
#include <iostream>
#include <cxxabi.h>


class Subscriber
{
private:
  Subscriber(std::function<void(const std::experimental::any&)> &&func, std::function<void()> &&cleanup) :
    m_func(std::move(func)), m_cleanup(std::move(cleanup)){}

public:
  using Ptr = std::shared_ptr<Subscriber>;

  ~Subscriber()
  {
    m_cleanup();
  }

  static Ptr create(
    std::function<void(const std::experimental::any&)> &&func,
    std::function<void()> &&cleanup
  )
  {
    return Ptr(new Subscriber(std::move(func),std::move(cleanup)));
  }

  void pub(const std::experimental::any& a)
  {
    m_func(a);
  };
private:
  std::function<void(const std::experimental::any&)> m_func;
  std::function<void()> m_cleanup;
};

class Node : public std::enable_shared_from_this<Node>
{
private:
  Node(std::function<void()> &&cleanup) : m_cleanup(std::move(cleanup)){}

public:
  using Ptr = std::shared_ptr<Node>;

  ~Node(){m_cleanup();}
  
  template <typename ARG>
  Subscriber::Ptr createSubscriber(typename std::function<void(const ARG&)> &&func)
  {
    std::lock_guard<std::mutex> lock(m_mutex);
    m_subs.emplace_back();
    auto it = m_subs.end();
    it--;
    auto capturedShared = shared_from_this();
    auto newNode =
      Subscriber::create(
        [=](const std::experimental::any &anyArg)
        {
          try
          {
            func(std::experimental::any_cast<const ARG &>(anyArg));
          }
          catch(std::experimental::fundamentals_v1::bad_any_cast &)
          {
            int status;
            std::cerr << "Subscriber has wrong type: " << abi::__cxa_demangle(typeid(ARG).name(),0,0,&status) << std::endl;
          }
        },
        //cleanup function  
        [=] {
          // list operations don't invalidate iterators
          // This iterator capture will be valid
          capturedShared->cleanupSubscriber(it);
        }
      );
    *it = newNode;
    return newNode;
  }

  void publish(const std::experimental::any &any)
  {
     std::lock_guard<std::mutex> lock(m_mutex);
     for(auto &sub : m_subs)
     {
       sub.lock()->pub(any);
     }
  }


  void cleanupSubscriber(std::list<std::weak_ptr<Subscriber>>::iterator it)
  {
    std::lock_guard<std::mutex> lock(m_mutex);
    m_subs.erase(it);
  }

  template <typename CLEANUP>
  static Ptr create(CLEANUP &&cleanup)
  {
    return Ptr(new Node(std::forward<CLEANUP>(cleanup)));
  }

private:
  std::mutex m_mutex;
  std::list<std::weak_ptr<Subscriber>> m_subs;

  std::function<void()> m_cleanup;
};

class Directory : public std::enable_shared_from_this<Directory>
{
private:
  Directory(){}

public:
  using Ptr = std::shared_ptr<Directory>;

  std::unordered_map<std::string, std::weak_ptr<Node>> m_mapService;

  template<typename PATH>
  Node::Ptr getNode(PATH &&path)
  {
    std::lock_guard<std::mutex> lock(m_mutex);
    auto it = m_mapService.find(path);
    if(it != m_mapService.end())
    {
      auto ptr = it->second.lock();
      if(ptr) return ptr;
    }
    
    auto capturedShared = shared_from_this();
    auto newNode = Node::create([=](){capturedShared->cleanupNode(path);});
    m_mapService.insert({path, newNode});
    return newNode;
  }

  void cleanupNode(const std::string &key)
  {
    std::lock_guard<std::mutex> lock(m_mutex);
    m_mapService.erase(m_mapService.find(key));
  }

  static Ptr create(){return Ptr(new Directory);}

private:
  std::mutex m_mutex;
};

class Foo {};
int main()
{
  auto dir = Directory::create();
  {
    auto sub = dir->getNode("/foo/bar")->createSubscriber(
      std::function<void(const Foo&)>(
      [](const Foo &obj) -> void
      {
        std::cout << "got foo" << std::endl;
      })
    );

    auto sub2 = dir->getNode("/foo/bar")->createSubscriber(
      std::function<void(const int&)>(
      [](const int &obj) -> void
      {
        std::cout << "got int?" << std::endl;
      }));
    dir->getNode("/foo/bar")->publish(Foo());
  }
  //The subscriber was destroyed, so they unsubscribed
  dir->getNode("/foo/bar")->publish(Foo());
  return 0;
}

Everything should clean itself up when things go out of scope.

from mrpt.

Simply wonderful :-)
I've been always uncomfortable with the waste of resources that imply marshaling objects through ZMQ / Sockets / ROS links. I guess your motivation was also in this line... We called this idea (in our former research lab) a "monolithic" robotic application, vs. distributed (i.e. multi-process) ones, which are easier, more robust but also far less efficient.

I think this could fit perfectly into the new MRPT 2.0 module mrpt-comms... 👍 (cc: #466 )

PS: shouldn't this:

  sub.lock()->XXX()

by guarded with an if() on the returned shared_ptr<>, just in case the underlying pointer beneath the weak_ptr is dangling?

from mrpt.

from mrpt.

I think the guard would be good just incase, but I think the other locks make that case impossible.

from mrpt.

Lock persists until the rvalue goes out of scope.

Yes, sure, I probably expressed my doubt in the wrong way... I think your next answer really addresses my point:

I think the guard would be good just incase, but I think the other locks make that case impossible.

Perfect then, thanks!

BTW: Just tested that, apparently, the new STD thread replacements work perfectly, by running one of the apps that makes intensive inter-thread data passing (rawlog-grabber) with a real camera. More tests should be done, but it works flawlessly so far! 👍

from mrpt.

These things are done:

Drop mrpt::synch::CAtomicCounter in favor of C++11 std::atomic.
CPipe : avoid deprecated auto_ptr<>

from mrpt.

👍

BTW: I'm postponing /s/OLD/NEW/g stuff (like MRPT_OVERRIDE->override) to the release of v1.5.0, so new code does not have to be re-fixed...

from mrpt.

Also...
Consider replacing uint32_t enums with correct C++11 typed enums. E.g. in GNSS_BINARY_MSG_DEFINITION_START

from mrpt.

Consider replacing uint32_t enums with correct C++11 typed enums. E.g. in GNSS_BINARY_MSG_DEFINITION_START

Nope, this one was not done... it's pretty easy anyway so I just did it now and I'm waiting for the tests to pass before pushing...

from mrpt.

Hi guys,

After the little fix here it seems the 2.0 branch successfully compiles for all MRPT apps and libraries.

Do you agree with merging this branch to master *NOW, deleting the 2.0 branch, and continuing the regular development in master and other branches for specific features?

from mrpt.

That sounds good. Probably want to prepare the GSoC students.

from mrpt.

Support move semantics

We already have many unit tests to verify move ctor & move = work for many classes.
This item could be checked out when we add more tests, virtually to almost all classes for which move makes sense.

from mrpt.

FYI: Today I finally removed the branch mrpt-2.0-devel. I verified that a previous GH fork with changes in that branch, can open a Pull Request against master and git catches up. Cheers!

from mrpt.

Update: 98aaaed removes our home-made make_vector<> since the C++11 braced initialization is far better and more general.

from mrpt.

Point "CMake export" moved to a new issue #602 . It actually has nothing to do with C++ versions.

from mrpt.

Done (mostly) with

Simplify ctors via member initialization in headers, e.g. int a { 0 }; or int a = 0;

from mrpt.

Use weak_ptr for m_frame_tf API in ReactiveNav

I think the weak_ptr/shared_ptr muddies the interface a little all to avoids the somewhat trivial copy.

You could pass it in as a parameter as well. That should avoid the copy as well.
What do you think of passing it as an optional param like nav.navigationStep(frameTF)? I guess the biggest problem that is you'd have to change a few signatures.

navigationStep(frameTF)->performNavigationStepNavigating(true, frameTF)->performNavigationStep(frameTF)
navigationStep(frameTF)->waypoints_navigationStep(frameTF)->performNavigationStepNavigating(false,frameTF)->performNavigationStep(frameTF)

from mrpt.

I think the weak_ptr/shared_ptr muddies the interface a little all to avoids the somewhat trivial copy.

I also had doubts about whether it's worth. Anyway, there should be no need for signature changes, unless I misunderstood you, since we already have void setFrameTF (mrpt::poses::FrameTransformer< 2 > *frame_tf). That pointer is the one that I was not 100% comfortable with. I think it might be only edge cases where the weak_ptr are required, since in a normal app lifecycle I don't imagine a situation when the user destroys the TF object while the RNAV engine is still alive.
Worth a thought...

from mrpt.

On this one (almost the last one that remains in this ticket!):

refactor type traits

I remember is what a @bergercookie 's thing related to traits of poses, but I don't know if that code is still around and where it is, etc. Any update on that part? I'll be happy of refactoring it, but just wanted to be sure it's still needed ;-)

from mrpt.

Make sure all move ctors and move = operators are declared noexcept. Otherwise, they will not be eligible for "real move".

I'm marking this one as done after reviewing that the condition is fulfilled for "lightweight" and "regular" point and poses, and for matrices.

from mrpt.

Which type traits?

from mrpt.

Which type traits?

That's the point... perhaps I'm making it up, but I could swear Nikos added some trait classes to the old mrpt-base module and we all agreed they could be refactored someday, but I just can't locate them now :-)

from mrpt.

Use weak_ptr for m_frame_tf API in ReactiveNav
done.

On the mysterious missing "traits", I'll just mark it as done and if @bergercookie thinks it's something still in the to-do list, it could be added to #697.

So, after almost 4 years, I think we can consider this task list as done... yay!! 🎉

from mrpt.

https://github.com/MRPT/mrpt/blob/80c44e1114fb113b31b56577eec7b80fb48402e6/libs/graphslam/include/mrpt/graphslam/types.h

from mrpt.

https://github.com/MRPT/mrpt/blob/80c44e1114fb113b31b56577eec7b80fb48402e6/libs/graphslam/include/mrpt/graphslam/types.h

Thanks, that one was on my radar, but it is fine as is: it defines graphslam-specific traits. The one I recall was related to poses and my thoughts then were that it seemed easily merge-able with mrpt::poses::SE_traits<> (?).

from mrpt.