[{"id":369,"web_url":"https://patchwork.libcamera.org/comment/369/","msgid":"<20190116152002.GG6484@bigcity.dyn.berto.se>","date":"2019-01-16T15:20:02","subject":"Re: [libcamera-devel] [PATCH v2 3/5] libcamera: media_object: Set\n\tdevnode in MediaEntity","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your patch.\n\nOn 2019-01-16 14:59:47 +0100, Jacopo Mondi wrote:\n> The MediaEntity::setDeviceNode() function was designed to set the device\n> node path associated with a MediaEntity. The function was there, but the\n> devnode_ member field was never actually set. Fix this.\n> \n> While at there add a getter method for the devnode_ member as it will\n> soon be used.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n\nI would have split this in two patches at it clearly does two different \nthings. For small patches like this it's not a huge issue but as patch \nsize grows the cost of review increases, at least for me.\n\nAlso with smaller patches you can apply from the bottom once they \nreceived proper review and decrease the size of the next version of a \nseries and release fixes earlier to master, if appropriate of course.\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  src/libcamera/include/media_object.h |  1 +\n>  src/libcamera/media_object.cpp       | 11 +++++++++++\n>  2 files changed, 12 insertions(+)\n> \n> diff --git a/src/libcamera/include/media_object.h b/src/libcamera/include/media_object.h\n> index a10f7e1..fad55a0 100644\n> --- a/src/libcamera/include/media_object.h\n> +++ b/src/libcamera/include/media_object.h\n> @@ -85,6 +85,7 @@ class MediaEntity : public MediaObject\n>  public:\n>  \tconst std::string &name() const { return name_; }\n>  \tunsigned int function() const { return function_; }\n> +\tconst std::string &devnode() const { return devnode_; }\n>  \tunsigned int deviceMajor() const { return major_; }\n>  \tunsigned int deviceMinor() const { return minor_; }\n>  \n> diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp\n> index 76dd326..4e90443 100644\n> --- a/src/libcamera/media_object.cpp\n> +++ b/src/libcamera/media_object.cpp\n> @@ -263,6 +263,15 @@ void MediaPad::addLink(MediaLink *link)\n>   * \\return The entity's function\n>   */\n>  \n> +/**\n> + * \\fn MediaEntity::devnode()\n> + * \\brief Retrieve the entity's device node path, if any\n> + *\n> + * \\sa int MediaEntity::setDeviceNode(const std::string &devnode)\n> + *\n> + * \\return The entity's device node path, or an empty string if it is not set\n> + */\n> +\n>  /**\n>   * \\fn MediaEntity::deviceMajor()\n>   * \\brief Retrieve the major number of the interface associated with the entity\n> @@ -330,6 +339,8 @@ int MediaEntity::setDeviceNode(const std::string &devnode)\n>  \t\treturn ret;\n>  \t}\n>  \n> +\tdevnode_ = devnode;\n> +\n>  \treturn 0;\n>  }\n>  \n> -- \n> 2.20.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3102760B2D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 16 Jan 2019 16:20:04 +0100 (CET)","by mail-lj1-x242.google.com with SMTP id g11-v6so5774197ljk.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 16 Jan 2019 07:20:04 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tg15sm1220432lfb.1.2019.01.16.07.20.02\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tWed, 16 Jan 2019 07:20:02 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=gPyFRgZaM5jmJjZuATGRWbXAZlCwEdrRPyweZuPtmy0=;\n\tb=l21ZoF647RCZj4xXOcv5cO5CrUyK/iPe508mT+mYyFNDHI8HCowO865dKtonGHi6Uw\n\tDYgO4NMfs7QDeA+hrnRUKw6JatZ5y51z9OTQoDSs1kbNBINhv1sJI9hZIjNbzPR+t17M\n\tMi4iBojXi0MGoFWntYPbHZmK/sK+nm5UL/RfaUvchNhxGPJuTfSkrRb5MbzO0dWCuWdq\n\tCOGKqMWUD7rA9kB94YdQYvI4olKcG4ydwtL2WZHARUWSPNG4eCkYFkwCw53RXqTVG6He\n\tR7IXaiufL6eKdHZnDel58aTpCKqc8MBy5sklHyhefqw2kQaqMTNSYMt9uWSdKvBNV5x3\n\tgF+Q==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=gPyFRgZaM5jmJjZuATGRWbXAZlCwEdrRPyweZuPtmy0=;\n\tb=kLzFO1oUbw3tyZ5LNWUDqSDBqsSFnsDqlN2oDJgji5APv9dtYQhxpyi+6JN61wgq9S\n\tb1P+sMRTgTukz8NisDgNt48VZK7Pn3o0bVnXp4WYe4/kgLlCW+M6pDQc3/wF0WDS8ul0\n\tkqlDGR4sXuRJRI02ve7fAZfN9iBwIaYY372Nzq2yj8fObToem8YQ+Xdj01X1222MOvjH\n\tBBcPn/Z/OW5DcYz94ExDYTRQE64uFEtEeLF1yD8U20MoxINCmiLrSQ8J3futYQWX1Lia\n\t/Hh8l9NPmxJjDYgIj27UOOCRAYDiOhBvYO2JKlK3Qn4lrR1M+T2MPScx25IdzIlrgJOa\n\tVP0w==","X-Gm-Message-State":"AJcUukec1jWtkiQybB36qtEkIs/kdRWq31nsSTSvbceMLvScDmGTmPus\n\tT3R8UenVFC/43StRgVpMUumlUrBBGH4=","X-Google-Smtp-Source":"ALg8bN6CZ/4VjKGF2Qe06v9uPg0k1Cafi1T9dEPFnq9a6cQdZyY/XumZEzOZD0tV9I5iY7FzLeYSrg==","X-Received":"by 2002:a2e:8045:: with SMTP id\n\tp5-v6mr6803492ljg.87.1547652003380; \n\tWed, 16 Jan 2019 07:20:03 -0800 (PST)","Date":"Wed, 16 Jan 2019 16:20:02 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190116152002.GG6484@bigcity.dyn.berto.se>","References":"<20190116135949.2097-1-jacopo@jmondi.org>\n\t<20190116135949.2097-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190116135949.2097-4-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 3/5] libcamera: media_object: Set\n\tdevnode in MediaEntity","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 16 Jan 2019 15:20:04 -0000"}},{"id":401,"web_url":"https://patchwork.libcamera.org/comment/401/","msgid":"<20190118010957.GL23244@pendragon.ideasonboard.com>","date":"2019-01-18T01:09:57","subject":"Re: [libcamera-devel] [PATCH v2 3/5] libcamera: media_object: Set\n\tdevnode in MediaEntity","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Wed, Jan 16, 2019 at 02:59:47PM +0100, Jacopo Mondi wrote:\n> The MediaEntity::setDeviceNode() function was designed to set the device\n> node path associated with a MediaEntity. The function was there, but the\n> devnode_ member field was never actually set. Fix this.\n> \n> While at there add a getter method for the devnode_ member as it will\n> soon be used.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  src/libcamera/include/media_object.h |  1 +\n>  src/libcamera/media_object.cpp       | 11 +++++++++++\n>  2 files changed, 12 insertions(+)\n> \n> diff --git a/src/libcamera/include/media_object.h b/src/libcamera/include/media_object.h\n> index a10f7e1..fad55a0 100644\n> --- a/src/libcamera/include/media_object.h\n> +++ b/src/libcamera/include/media_object.h\n> @@ -85,6 +85,7 @@ class MediaEntity : public MediaObject\n>  public:\n>  \tconst std::string &name() const { return name_; }\n>  \tunsigned int function() const { return function_; }\n> +\tconst std::string &devnode() const { return devnode_; }\n\nAs the setter is called setDeviceNode(), should this be called\ndeviceNode() ? We usually try not to abbreviate when not necessary.\n\n>  \tunsigned int deviceMajor() const { return major_; }\n>  \tunsigned int deviceMinor() const { return minor_; }\n>  \n> diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp\n> index 76dd326..4e90443 100644\n> --- a/src/libcamera/media_object.cpp\n> +++ b/src/libcamera/media_object.cpp\n> @@ -263,6 +263,15 @@ void MediaPad::addLink(MediaLink *link)\n>   * \\return The entity's function\n>   */\n>  \n> +/**\n> + * \\fn MediaEntity::devnode()\n> + * \\brief Retrieve the entity's device node path, if any\n> + *\n> + * \\sa int MediaEntity::setDeviceNode(const std::string &devnode)\n\nI think you can abbreviate that to \\sa setDeviceNode()\n\nWith these addressed,\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> + *\n> + * \\return The entity's device node path, or an empty string if it is not set\n> + */\n> +\n>  /**\n>   * \\fn MediaEntity::deviceMajor()\n>   * \\brief Retrieve the major number of the interface associated with the entity\n> @@ -330,6 +339,8 @@ int MediaEntity::setDeviceNode(const std::string &devnode)\n>  \t\treturn ret;\n>  \t}\n>  \n> +\tdevnode_ = devnode;\n> +\n>  \treturn 0;\n>  }\n>","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 48A8160C83\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 18 Jan 2019 02:09:58 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 82CE653F;\n\tFri, 18 Jan 2019 02:09:57 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1547773797;\n\tbh=aWb/BI0oBo/GcgIVmrgvhzeIx0bi/8Pa3LOiZwiMsfk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=n1r0rnKyTXMZAtEc60iUKWCvVQ4kPeuNg/cjkTRFC5O2zCoczLw/b/P22EK5b4J/D\n\tzgyTOC+m1CT2jiXGq9Agl+plVdtFONz/UGU79A+H+vwAH/fCdMUVBIDLWT3K8M31VK\n\tT6+lexnA3g7aQyZ+yhV/iTCNpAZwWz510VuKNklw=","Date":"Fri, 18 Jan 2019 03:09:57 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190118010957.GL23244@pendragon.ideasonboard.com>","References":"<20190116135949.2097-1-jacopo@jmondi.org>\n\t<20190116135949.2097-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190116135949.2097-4-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 3/5] libcamera: media_object: Set\n\tdevnode in MediaEntity","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Fri, 18 Jan 2019 01:09:58 -0000"}},{"id":438,"web_url":"https://patchwork.libcamera.org/comment/438/","msgid":"<20190121102745.dimfn6y7hx56ar56@uno.localdomain>","date":"2019-01-21T10:27:45","subject":"Re: [libcamera-devel] [PATCH v2 3/5] libcamera: media_object: Set\n\tdevnode in MediaEntity","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Fri, Jan 18, 2019 at 03:09:57AM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Wed, Jan 16, 2019 at 02:59:47PM +0100, Jacopo Mondi wrote:\n> > The MediaEntity::setDeviceNode() function was designed to set the device\n> > node path associated with a MediaEntity. The function was there, but the\n> > devnode_ member field was never actually set. Fix this.\n> >\n> > While at there add a getter method for the devnode_ member as it will\n> > soon be used.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> >  src/libcamera/include/media_object.h |  1 +\n> >  src/libcamera/media_object.cpp       | 11 +++++++++++\n> >  2 files changed, 12 insertions(+)\n> >\n> > diff --git a/src/libcamera/include/media_object.h b/src/libcamera/include/media_object.h\n> > index a10f7e1..fad55a0 100644\n> > --- a/src/libcamera/include/media_object.h\n> > +++ b/src/libcamera/include/media_object.h\n> > @@ -85,6 +85,7 @@ class MediaEntity : public MediaObject\n> >  public:\n> >  \tconst std::string &name() const { return name_; }\n> >  \tunsigned int function() const { return function_; }\n> > +\tconst std::string &devnode() const { return devnode_; }\n>\n> As the setter is called setDeviceNode(), should this be called\n> deviceNode() ? We usually try not to abbreviate when not necessary.\n>\n\nI would like to have devnode() as the MediaDevice exposes a function\nwith the same purpose namde like that. Or I either change both or stay\nwith devnode() here. Is this a big deal?\n\n\n> >  \tunsigned int deviceMajor() const { return major_; }\n> >  \tunsigned int deviceMinor() const { return minor_; }\n> >\n> > diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp\n> > index 76dd326..4e90443 100644\n> > --- a/src/libcamera/media_object.cpp\n> > +++ b/src/libcamera/media_object.cpp\n> > @@ -263,6 +263,15 @@ void MediaPad::addLink(MediaLink *link)\n> >   * \\return The entity's function\n> >   */\n> >\n> > +/**\n> > + * \\fn MediaEntity::devnode()\n> > + * \\brief Retrieve the entity's device node path, if any\n> > + *\n> > + * \\sa int MediaEntity::setDeviceNode(const std::string &devnode)\n>\n> I think you can abbreviate that to \\sa setDeviceNode()\n>\n> With these addressed,\n>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>\n\nThanks\n  j\n\n> > + *\n> > + * \\return The entity's device node path, or an empty string if it is not set\n> > + */\n> > +\n> >  /**\n> >   * \\fn MediaEntity::deviceMajor()\n> >   * \\brief Retrieve the major number of the interface associated with the entity\n> > @@ -330,6 +339,8 @@ int MediaEntity::setDeviceNode(const std::string &devnode)\n> >  \t\treturn ret;\n> >  \t}\n> >\n> > +\tdevnode_ = devnode;\n> > +\n> >  \treturn 0;\n> >  }\n> >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 79FB460C65\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 21 Jan 2019 11:27:33 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 111961BF219;\n\tMon, 21 Jan 2019 10:27:32 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Mon, 21 Jan 2019 11:27:45 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190121102745.dimfn6y7hx56ar56@uno.localdomain>","References":"<20190116135949.2097-1-jacopo@jmondi.org>\n\t<20190116135949.2097-4-jacopo@jmondi.org>\n\t<20190118010957.GL23244@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"lgbr4dz7eugba6fq\"","Content-Disposition":"inline","In-Reply-To":"<20190118010957.GL23244@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 3/5] libcamera: media_object: Set\n\tdevnode in MediaEntity","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 21 Jan 2019 10:27:33 -0000"}},{"id":445,"web_url":"https://patchwork.libcamera.org/comment/445/","msgid":"<20190121120818.GB12780@pendragon.ideasonboard.com>","date":"2019-01-21T12:08:18","subject":"Re: [libcamera-devel] [PATCH v2 3/5] libcamera: media_object: Set\n\tdevnode in MediaEntity","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Jan 21, 2019 at 11:27:45AM +0100, Jacopo Mondi wrote:\n>  On Fri, Jan 18, 2019 at 03:09:57AM +0200, Laurent Pinchart wrote:\n> > On Wed, Jan 16, 2019 at 02:59:47PM +0100, Jacopo Mondi wrote:\n> >> The MediaEntity::setDeviceNode() function was designed to set the device\n> >> node path associated with a MediaEntity. The function was there, but the\n> >> devnode_ member field was never actually set. Fix this.\n> >> \n> >> While at there add a getter method for the devnode_ member as it will\n> >> soon be used.\n> >> \n> >> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> >> ---\n> >> src/libcamera/include/media_object.h |  1 +\n> >> src/libcamera/media_object.cpp       | 11 +++++++++++\n> >> 2 files changed, 12 insertions(+)\n> >> \n> >> diff --git a/src/libcamera/include/media_object.h b/src/libcamera/include/media_object.h\n> >> index a10f7e1..fad55a0 100644\n> >> --- a/src/libcamera/include/media_object.h\n> >> +++ b/src/libcamera/include/media_object.h\n> >> @@ -85,6 +85,7 @@ class MediaEntity : public MediaObject\n> >> public:\n> >> \tconst std::string &name() const { return name_; }\n> >> \tunsigned int function() const { return function_; }\n> >> +\tconst std::string &devnode() const { return devnode_; }\n> > \n> > As the setter is called setDeviceNode(), should this be called\n> > deviceNode() ? We usually try not to abbreviate when not necessary.\n>  \n>  I would like to have devnode() as the MediaDevice exposes a function\n>  with the same purpose namde like that. Or I either change both or stay\n>  with devnode() here. Is this a big deal?\n\nI think consistency is important. I'm OK with both devnode and\ndeviceNode (possibly with a preference for the latter), so you can pick\nthe one you like best, but the getter and setter in MediaEntity should\nbe consistent. If they can be consistent with MediaDevice it's probably\nbest.\n\n> >> \tunsigned int deviceMajor() const { return major_; }\n> >> \tunsigned int deviceMinor() const { return minor_; }\n\nThis is why I prefer deviceNode() over devnode() by the way, to be\nconsistent with these two functions. I'll let you decide which one you\nprefer.\n\n> >> diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp\n> >> index 76dd326..4e90443 100644\n> >> --- a/src/libcamera/media_object.cpp\n> >> +++ b/src/libcamera/media_object.cpp\n> >> @@ -263,6 +263,15 @@ void MediaPad::addLink(MediaLink *link)\n> >> * \\return The entity's function\n> >> */\n> >> \n> >> +/**\n> >> + * \\fn MediaEntity::devnode()\n> >> + * \\brief Retrieve the entity's device node path, if any\n> >> + *\n> >> + * \\sa int MediaEntity::setDeviceNode(const std::string &devnode)\n> > \n> > I think you can abbreviate that to \\sa setDeviceNode()\n> > \n> > With these addressed,\n> > \n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > \n> >> + *\n> >> + * \\return The entity's device node path, or an empty string if it is not set\n> >> + */\n> >> +\n> >> /**\n> >> * \\fn MediaEntity::deviceMajor()\n> >> * \\brief Retrieve the major number of the interface associated with the entity\n> >> @@ -330,6 +339,8 @@ int MediaEntity::setDeviceNode(const std::string &devnode)\n> >> \t\treturn ret;\n> >> \t}\n> >> \n> >> +\tdevnode_ = devnode;\n> >> +\n> >> \treturn 0;\n> >> }\n> >>","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 22BAB60C7F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 21 Jan 2019 13:08:19 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yyyyyyyyyyyyyby-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00::2])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 82AAF53E;\n\tMon, 21 Jan 2019 13:08:18 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548072498;\n\tbh=l7QSG+eEiVJ4KhXbYLSgqn95EblkbwbIYmKj0tW7hVI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=F+2pkbi6jBaWrk5ns0pAnAjmqi/wa73FhwdDfP/H6PRYsTX6sEVF43gAabsplF8pf\n\taTczWcJL/o1mCX0cqeGxWYtE0V2xaVYKUa4oa5sdEvsKDAwGa4ODlIfEdGOOsA8AC2\n\tJVcRE0U90zHNr71ydW+h/ZFQMz4Z1wiy/b/kMvOU=","Date":"Mon, 21 Jan 2019 14:08:18 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190121120818.GB12780@pendragon.ideasonboard.com>","References":"<20190116135949.2097-1-jacopo@jmondi.org>\n\t<20190116135949.2097-4-jacopo@jmondi.org>\n\t<20190118010957.GL23244@pendragon.ideasonboard.com>\n\t<20190121102745.dimfn6y7hx56ar56@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190121102745.dimfn6y7hx56ar56@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 3/5] libcamera: media_object: Set\n\tdevnode in MediaEntity","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 21 Jan 2019 12:08:19 -0000"}}]