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

1701 diff mbox series

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@jmondi.org>
---
 src/libcamera/include/message.h |  8 ++++++++
 src/libcamera/message.cpp       | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+)

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@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

Hi Jacopo,

On Mon, Jul 15, 2019 at 11:59:44AM +0300, Laurent Pinchart wrote:
> 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@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.

Nevermind splitting it to another patch, it's fine to combine them, but
it should still be documented.

> >  	};
> >  
> >  	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
>