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

Message ID 20190715092939.15349-2-laurent.pinchart@ideasonboard.com
State Accepted
Commit fae053307dcc6807dd8ab127294c1fe5c5bb2d72
Headers show
Series
  • [libcamera-devel,1/2] libcamera: message: Document Message::SignalMessage
Related show

Commit Message

Laurent Pinchart July 15, 2019, 9:29 a.m. UTC
From: Jacopo Mondi <jacopo@jmondi.org>

Reserve identifiers for user-defined message types and add an operation
to the Message class to create register the type identifiers.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v1:

- Replace mutex with atomic
- Rework documentation
- Rename registerUserMessageType() to registerMessageType()
- Extend the Message test

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/include/message.h |  7 +++++++
 src/libcamera/message.cpp       | 36 +++++++++++++++++++++++++++++++++
 test/message.cpp                | 11 ++++++++++
 3 files changed, 54 insertions(+)

Comments

Jacopo Mondi July 16, 2019, 7:39 a.m. UTC | #1
On Mon, Jul 15, 2019 at 12:29:39PM +0300, Laurent Pinchart wrote:
> From: Jacopo Mondi <jacopo@jmondi.org>
>
> Reserve identifiers for user-defined message types and add an operation
> to the Message class to create register the type identifiers.

create 'and' register ?

Otherwise
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Changes since v1:
>
> - Replace mutex with atomic
> - Rework documentation
> - Rename registerUserMessageType() to registerMessageType()
> - Extend the Message test
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/include/message.h |  7 +++++++
>  src/libcamera/message.cpp       | 36 +++++++++++++++++++++++++++++++++
>  test/message.cpp                | 11 ++++++++++
>  3 files changed, 54 insertions(+)
>
> diff --git a/src/libcamera/include/message.h b/src/libcamera/include/message.h
> index db17d647c280..416fe74b10ad 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 <atomic>
> +
>  namespace libcamera {
>
>  class Object;
> @@ -19,6 +21,7 @@ public:
>  	enum Type {
>  		None = 0,
>  		SignalMessage = 1,
> +		UserMessage = 1000,
>  	};
>
>  	Message(Type type);
> @@ -27,11 +30,15 @@ public:
>  	Type type() const { return type_; }
>  	Object *receiver() const { return receiver_; }
>
> +	static Type registerMessageType();
> +
>  private:
>  	friend class Thread;
>
>  	Type type_;
>  	Object *receiver_;
> +
> +	static std::atomic_uint nextUserType_;
>  };
>
>  class SignalMessage : public Message
> diff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp
> index 9f22ad7fc0b0..d44d2a4c73a8 100644
> --- a/src/libcamera/message.cpp
> +++ b/src/libcamera/message.cpp
> @@ -31,6 +31,8 @@ namespace libcamera {
>
>  LOG_DEFINE_CATEGORY(Message)
>
> +std::atomic_uint Message::nextUserType_{ Message::UserMessage };
> +
>  /**
>   * \class Message
>   * \brief A message that can be posted to a Thread
> @@ -43,6 +45,8 @@ LOG_DEFINE_CATEGORY(Message)
>   * \brief Invalid message type
>   * \var Message::SignalMessage
>   * \brief Asynchronous signal delivery across threads
> + * \var Message::UserMessage
> + * \brief First value available for user-defined messages
>   */
>
>  /**
> @@ -70,6 +74,38 @@ Message::~Message()
>   * \return The message receiver
>   */
>
> +/**
> + * \brief Reserve and register a custom user-defined message type
> + *
> + * Custom message types use values starting at Message::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::registerMessageType()
> +{
> +	return static_cast<Message::Type>(nextUserType_++);
> +}
> +
>  /**
>   * \class SignalMessage
>   * \brief A message carrying a Signal across threads
> diff --git a/test/message.cpp b/test/message.cpp
> index de98da3e8754..3775c30a20b3 100644
> --- a/test/message.cpp
> +++ b/test/message.cpp
> @@ -52,6 +52,17 @@ class MessageTest : public Test
>  protected:
>  	int run()
>  	{
> +		Message::Type msgType[2] = {
> +			Message::registerMessageType(),
> +			Message::registerMessageType(),
> +		};
> +
> +		if (msgType[0] != Message::UserMessage ||
> +		    msgType[1] != Message::UserMessage + 1) {
> +			cout << "Failed to register message types" << endl;
> +			return TestFail;
> +		}
> +
>  		MessageReceiver receiver;
>  		receiver.moveToThread(&thread_);
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart July 16, 2019, 7:55 a.m. UTC | #2
Hi Jacopo,

On Tue, Jul 16, 2019 at 09:39:24AM +0200, Jacopo Mondi wrote:
> On Mon, Jul 15, 2019 at 12:29:39PM +0300, Laurent Pinchart wrote:
> > From: Jacopo Mondi <jacopo@jmondi.org>
> >
> > Reserve identifiers for user-defined message types and add an operation
> > to the Message class to create register the type identifiers.
> 
> create 'and' register ?

Oops :-) Will fix.

> Otherwise
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > Changes since v1:
> >
> > - Replace mutex with atomic
> > - Rework documentation
> > - Rename registerUserMessageType() to registerMessageType()
> > - Extend the Message test
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/libcamera/include/message.h |  7 +++++++
> >  src/libcamera/message.cpp       | 36 +++++++++++++++++++++++++++++++++
> >  test/message.cpp                | 11 ++++++++++
> >  3 files changed, 54 insertions(+)
> >
> > diff --git a/src/libcamera/include/message.h b/src/libcamera/include/message.h
> > index db17d647c280..416fe74b10ad 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 <atomic>
> > +
> >  namespace libcamera {
> >
> >  class Object;
> > @@ -19,6 +21,7 @@ public:
> >  	enum Type {
> >  		None = 0,
> >  		SignalMessage = 1,
> > +		UserMessage = 1000,
> >  	};
> >
> >  	Message(Type type);
> > @@ -27,11 +30,15 @@ public:
> >  	Type type() const { return type_; }
> >  	Object *receiver() const { return receiver_; }
> >
> > +	static Type registerMessageType();
> > +
> >  private:
> >  	friend class Thread;
> >
> >  	Type type_;
> >  	Object *receiver_;
> > +
> > +	static std::atomic_uint nextUserType_;
> >  };
> >
> >  class SignalMessage : public Message
> > diff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp
> > index 9f22ad7fc0b0..d44d2a4c73a8 100644
> > --- a/src/libcamera/message.cpp
> > +++ b/src/libcamera/message.cpp
> > @@ -31,6 +31,8 @@ namespace libcamera {
> >
> >  LOG_DEFINE_CATEGORY(Message)
> >
> > +std::atomic_uint Message::nextUserType_{ Message::UserMessage };
> > +
> >  /**
> >   * \class Message
> >   * \brief A message that can be posted to a Thread
> > @@ -43,6 +45,8 @@ LOG_DEFINE_CATEGORY(Message)
> >   * \brief Invalid message type
> >   * \var Message::SignalMessage
> >   * \brief Asynchronous signal delivery across threads
> > + * \var Message::UserMessage
> > + * \brief First value available for user-defined messages
> >   */
> >
> >  /**
> > @@ -70,6 +74,38 @@ Message::~Message()
> >   * \return The message receiver
> >   */
> >
> > +/**
> > + * \brief Reserve and register a custom user-defined message type
> > + *
> > + * Custom message types use values starting at Message::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::registerMessageType()
> > +{
> > +	return static_cast<Message::Type>(nextUserType_++);
> > +}
> > +
> >  /**
> >   * \class SignalMessage
> >   * \brief A message carrying a Signal across threads
> > diff --git a/test/message.cpp b/test/message.cpp
> > index de98da3e8754..3775c30a20b3 100644
> > --- a/test/message.cpp
> > +++ b/test/message.cpp
> > @@ -52,6 +52,17 @@ class MessageTest : public Test
> >  protected:
> >  	int run()
> >  	{
> > +		Message::Type msgType[2] = {
> > +			Message::registerMessageType(),
> > +			Message::registerMessageType(),
> > +		};
> > +
> > +		if (msgType[0] != Message::UserMessage ||
> > +		    msgType[1] != Message::UserMessage + 1) {
> > +			cout << "Failed to register message types" << endl;
> > +			return TestFail;
> > +		}
> > +
> >  		MessageReceiver receiver;
> >  		receiver.moveToThread(&thread_);
> >

Patch

diff --git a/src/libcamera/include/message.h b/src/libcamera/include/message.h
index db17d647c280..416fe74b10ad 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 <atomic>
+
 namespace libcamera {
 
 class Object;
@@ -19,6 +21,7 @@  public:
 	enum Type {
 		None = 0,
 		SignalMessage = 1,
+		UserMessage = 1000,
 	};
 
 	Message(Type type);
@@ -27,11 +30,15 @@  public:
 	Type type() const { return type_; }
 	Object *receiver() const { return receiver_; }
 
+	static Type registerMessageType();
+
 private:
 	friend class Thread;
 
 	Type type_;
 	Object *receiver_;
+
+	static std::atomic_uint nextUserType_;
 };
 
 class SignalMessage : public Message
diff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp
index 9f22ad7fc0b0..d44d2a4c73a8 100644
--- a/src/libcamera/message.cpp
+++ b/src/libcamera/message.cpp
@@ -31,6 +31,8 @@  namespace libcamera {
 
 LOG_DEFINE_CATEGORY(Message)
 
+std::atomic_uint Message::nextUserType_{ Message::UserMessage };
+
 /**
  * \class Message
  * \brief A message that can be posted to a Thread
@@ -43,6 +45,8 @@  LOG_DEFINE_CATEGORY(Message)
  * \brief Invalid message type
  * \var Message::SignalMessage
  * \brief Asynchronous signal delivery across threads
+ * \var Message::UserMessage
+ * \brief First value available for user-defined messages
  */
 
 /**
@@ -70,6 +74,38 @@  Message::~Message()
  * \return The message receiver
  */
 
+/**
+ * \brief Reserve and register a custom user-defined message type
+ *
+ * Custom message types use values starting at Message::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::registerMessageType()
+{
+	return static_cast<Message::Type>(nextUserType_++);
+}
+
 /**
  * \class SignalMessage
  * \brief A message carrying a Signal across threads
diff --git a/test/message.cpp b/test/message.cpp
index de98da3e8754..3775c30a20b3 100644
--- a/test/message.cpp
+++ b/test/message.cpp
@@ -52,6 +52,17 @@  class MessageTest : public Test
 protected:
 	int run()
 	{
+		Message::Type msgType[2] = {
+			Message::registerMessageType(),
+			Message::registerMessageType(),
+		};
+
+		if (msgType[0] != Message::UserMessage ||
+		    msgType[1] != Message::UserMessage + 1) {
+			cout << "Failed to register message types" << endl;
+			return TestFail;
+		}
+
 		MessageReceiver receiver;
 		receiver.moveToThread(&thread_);