[libcamera-devel] [PATCH 3/3] libcamera: message: Add user message types

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Mon, Jul 15, 2019 at 07:59:35AM +0200, Jacopo Mondi wrote:
> Add an operation to the Message class to create unique message type to
> be used by derived classes to uniquely identify the messages they
> handle.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/include/message.h |  8 ++++++++
>  src/libcamera/message.cpp       | 20 ++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/src/libcamera/include/message.h b/src/libcamera/include/message.h
> index db17d647c280..e7dd85a606a3 100644
> --- a/src/libcamera/include/message.h
> +++ b/src/libcamera/include/message.h
> @@ -7,6 +7,8 @@
>  #ifndef __LIBCAMERA_MESSAGE_H__
>  #define __LIBCAMERA_MESSAGE_H__
>  
> +#include <mutex>
> +
>  namespace libcamera {
>  
>  class Object;
> @@ -19,6 +21,7 @@ public:
>  	enum Type {
>  		None = 0,
>  		SignalMessage = 1,
> +		UserMessage = 1000,

This should be documented, and it may make sense to split it to a
separate patch.

>  	};
>  
>  	Message(Type type);
> @@ -27,11 +30,16 @@ public:
>  	Type type() const { return type_; }
>  	Object *receiver() const { return receiver_; }
>  
> +	static Type registerUserMessageType();

I think we can shorten the name to registerMessageType().

> +
>  private:
>  	friend class Thread;
>  
>  	Type type_;
>  	Object *receiver_;
> +
> +	static std::mutex mutex_;
> +	static Type nextUserType_;

I wonder if we could use an atomic variable instead and remove the
mutex.

	static std::atomic_uint nextUserType_;

>  };
>  
>  class SignalMessage : public Message
> diff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp
> index 0580c1051622..f82a3115595d 100644
> --- a/src/libcamera/message.cpp
> +++ b/src/libcamera/message.cpp
> @@ -6,6 +6,7 @@
>   */
>  
>  #include "message.h"
> +#include "thread.h"
>  
>  #include "log.h"
>  
> @@ -31,6 +32,9 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(Message)
>  
> +std::mutex Message::mutex_;
> +Message::Type Message::nextUserType_ = Message::UserMessage;
> +
>  /**
>   * \class Message
>   * \brief A message that can be posted to a Thread
> @@ -68,6 +72,22 @@ Message::~Message()
>   * \return The message receiver
>   */
>  
> +/**
> + * \brief Retrieve a unique identifier for user defined message types

Reserve and register a custom user-defined message type

> + *
> + * Derived classes can define their own message types and this method provides
> + * them a unique identifier to use for that purpose.

How about this ?

 * Custom message types use values starting at Type::UserMessage. Assigning
 * custom types manually may lead to accidental duplicated types. To avoid this
 * problem, this method reserves and returns the next available user-defined
 * message type.
 *
 * The recommended way to use this method is to subclass Message and provide a
 * static accessor for the custom message type.
 *
 * \code{.cpp}
 * class MyCustomMessage : public Message
 * {
 * public:
 *      MyCustomMessage() : Message(type()) { }
 *
 *      static Message::Type type()
 *      {
 *              static MessageType type = registerMessageType();
 *              return type;
 *      }
 * };
 * \endcode

> + *
> + * \return A new unique message type
> + */
> +Message::Type Message::registerUserMessageType()
> +{
> +	MutexLocker locker(mutex_);
> +
> +	return nextUserType_ = static_cast<Type>
> +			      (static_cast<unsigned int>(nextUserType_) + 1);

	return static_cast<Message::Type>(nextUserType_++);

> +}
> +
>  /**
>   * \class SignalMessage
>   * \brief A message carrying a Signal across threads

-- 
Regards,

Laurent Pinchart