[{"id":203,"web_url":"https://patchwork.libcamera.org/comment/203/","msgid":"<20190104164730.GO22790@bigcity.dyn.berto.se>","date":"2019-01-04T16:47:30","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: Document private\n\tmembers","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 work.\n\nOn 2019-01-03 18:38:58 +0100, Jacopo Mondi wrote:\n> The newly added MediaDevice::setLink() function is defined as private,\n> but it is worth being documented, as it is called from the friend method\n> MediaLink::setup().\n> \n> In the library code base, several private methods and fields are\n> documented, but do not show up in the generated documentation output.\n> Change doxygen settings to output processed documentation for the\n> private fields and methods, when proper doxygen comments are applied to\n> them (and do not complain if private members are not documented at all).\n\nI would split this to a separate commit.\n\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> Documentation/Doxyfile.in | 4 ++--\n> src/libcamera/media_device.cpp | 22 ++++++++++++++++++++++\n> 2 files changed, 24 insertions(+), 2 deletions(-)\n> \n> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> index b1a70d3..16dcccd 100644\n> --- a/Documentation/Doxyfile.in\n> +++ b/Documentation/Doxyfile.in\n> @@ -442,7 +442,7 @@ EXTRACT_ALL = NO\n> # be included in the documentation.\n> # The default value is: NO.\n> \n> -EXTRACT_PRIVATE = NO\n> +EXTRACT_PRIVATE = YES\n> \n> # If the EXTRACT_PACKAGE tag is set to YES, all members with package or internal\n> # scope will be included in the documentation.\n> @@ -487,7 +487,7 @@ EXTRACT_ANON_NSPACES = NO\n> # section is generated. This option has no effect if EXTRACT_ALL is enabled.\n> # The default value is: NO.\n> \n> -HIDE_UNDOC_MEMBERS = NO\n> +HIDE_UNDOC_MEMBERS = YES\n> \n> # If the HIDE_UNDOC_CLASSES tag is set to YES, doxygen will hide all\n> # undocumented classes that are normally visible in the class hierarchy. If set\n> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp\n> index b86d0c4..48aa805 100644\n> --- a/src/libcamera/media_device.cpp\n> +++ b/src/libcamera/media_device.cpp\n> @@ -641,6 +641,28 @@ bool MediaDevice::populateLinks(const struct media_v2_topology &topology)\n> \treturn true;\n> }\n> \n> +/**\n> + * \\brief Apply \\a flags to a link between two pads\n> + * \\param source The source pad\n> + * \\param sink The sink pad\n> + * \\param flags The link flags\n> + *\n> + * Implements 'raw' link handling, as this functions applies \\a flags,\n> + * (whose only accepted values are the ones defined by the Media Controller\n> + * APIs in MEDIA_LNK_FL_* macros) to a link between two pads. No correctness\n> + * checks on the link existence and validity of \\a flags is performed. The\n> + * function assumes the \\a source and \\a sink pads are connected, and the\n> + * supplied \\a flags applies to the link (ie. immutable links cannot be\n> + * disabled).\n> + *\n> + * This function wraps the raw MEDIA_IOC_SETUP_LINK ioctl, and shouldn't be\n> + * called directly by any other class or method but the\n> + * MediaLink::setup(bool enable) one, declared as friend for this reason.\n\nI don't love this, make setLink() public :-)\n\n> + *\n> + * \\sa MediaLink::setup(bool enable)\n> + *\n> + * \\return 0 for success, negative error number otherwise\n> + */\n> int MediaDevice::setLink(const MediaPad *source, const MediaPad *sink,\n> \t\t\t unsigned int flags)\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":"","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 70BD860B0B\n\tfor ;\n\tFri, 4 Jan 2019 17:47:39 +0100 (CET)","by mail-lj1-x242.google.com with SMTP id g11-v6so32913842ljk.3\n\tfor ;\n\tFri, 04 Jan 2019 08:47:39 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tu11sm11408289lfb.85.2019.01.04.08.47.30\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 04 Jan 2019 08:47:31 -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=0YKBBViBT14ckuSO033Rt/aoc9ijgq87HUyOvsM8sNY=;\n\tb=c/sQfT69oFkQ7OAIuLR/kjqiZydMXyHV+upPWTMxme5FzL7olkJE1BfFKSg4pv8vgC\n\t14evsqjpdTY3jlUfq0giFgGcYKSLoeXFQv7eRd/Xwc0BcZ7GpYJbYPp4it7pKTr2AC/K\n\tIdjmY/C7DYdUbdX7ekkQb1rUcVuU6SW49HWDj9KvzEq4khk3Hhw0+M4Js33v3I17JnC2\n\tTg9sJ98gPG2nXESzTv/mkczgb69uBm6+lkksJUP2w6R2OSwUQytEHtALD9snOetJNHYM\n\tb62x749ini9ZqhC21NahxOkwAB8AuXNU0YRXgSkg1G1rrFaVK3OCoiDIifLP6wE56VGI\n\thihQ==","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=0YKBBViBT14ckuSO033Rt/aoc9ijgq87HUyOvsM8sNY=;\n\tb=tivayiyqDByBHDmda+ZwRcMslcVLvcvDtvNctVCKrych+cCL/eiWjzfdeW0Shpq4vN\n\tJ17fMB1kwNyMVKjgR/R599TN5GwOYDNpu6pAVwEBaVAjn4AInf5mFGIvRCBrU3SFpk3d\n\tcrkMcKeuh+DrzdZ6bGhGit9eVVCKMoGHHVfe2QAoi31lYxYdii+NBjjldFatJLxh+Izk\n\tWaiN6KKfFH2N66Z4k5HlVJmYCDaITk0/ful7xGby0UH5b3ba+NwA3z/atD08ZVB+E3a4\n\t6ccBELka2gTKxUIAsW3JQIWNWj/0zYJL28oxx5j4tzGDckWv7fkFxqrSqTzl25WTJBy3\n\tRGlg==","X-Gm-Message-State":"AJcUukfF78rnPkSvHaALU9/BY1S9NPNeNrZtDIBuX5wlpkPy74Lb8mVj\n\tiGVEmq0GaykTpJvSNcTVn5yx+w==","X-Google-Smtp-Source":"ALg8bN5lbDPxEldqXLVag5mZaQEkyoWSverRftUjYQVGGHS9zEtYEf2QyqjyBUbKmG/EgQ6gnWlNFA==","X-Received":"by 2002:a2e:3308:: with SMTP id\n\td8-v6mr24421331ljc.38.1546620451660; \n\tFri, 04 Jan 2019 08:47:31 -0800 (PST)","Date":"Fri, 4 Jan 2019 17:47:30 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190104164730.GO22790@bigcity.dyn.berto.se>","References":"<20190103173859.22624-1-jacopo@jmondi.org>\n\t<20190103173859.22624-5-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":"<20190103173859.22624-5-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: Document private\n\tmembers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 04 Jan 2019 16:47:39 -0000"}},{"id":238,"web_url":"https://patchwork.libcamera.org/comment/238/","msgid":"<6022267.4GnW3kY2sG@avalon>","date":"2019-01-07T21:58:16","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: Document private\n\tmembers","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Friday, 4 January 2019 18:47:30 EET Niklas Söderlund wrote:\n> On 2019-01-03 18:38:58 +0100, Jacopo Mondi wrote:\n> > The newly added MediaDevice::setLink() function is defined as private,\n> > but it is worth being documented, as it is called from the friend method\n> > MediaLink::setup().\n> > \n> > In the library code base, several private methods and fields are\n> > documented, but do not show up in the generated documentation output.\n> > Change doxygen settings to output processed documentation for the\n> > private fields and methods, when proper doxygen comments are applied to\n> > them (and do not complain if private members are not documented at all).\n> \n> I would split this to a separate commit.\n\nI would also move documentation of setLink() to patch 3/5.\n\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > \n> > Documentation/Doxyfile.in | 4 ++--\n> > src/libcamera/media_device.cpp | 22 ++++++++++++++++++++++\n> > 2 files changed, 24 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> > index b1a70d3..16dcccd 100644\n> > --- a/Documentation/Doxyfile.in\n> > +++ b/Documentation/Doxyfile.in\n> > @@ -442,7 +442,7 @@ EXTRACT_ALL = NO\n> > \n> > # be included in the documentation.\n> > # The default value is: NO.\n> > \n> > -EXTRACT_PRIVATE = NO\n> > +EXTRACT_PRIVATE = YES\n> > \n> > # If the EXTRACT_PACKAGE tag is set to YES, all members with package or\n> > internal # scope will be included in the documentation.\n> > \n> > @@ -487,7 +487,7 @@ EXTRACT_ANON_NSPACES = NO\n> > \n> > # section is generated. This option has no effect if EXTRACT_ALL is\n> > enabled. # The default value is: NO.\n> > \n> > -HIDE_UNDOC_MEMBERS = NO\n> > +HIDE_UNDOC_MEMBERS = YES\n\nThis will remove doxygen warnings for undocumented private members, which \nachieves what you were trying to doc, but it will also remove those warnings \nfor public members, which is really bad.\n\n> > # If the HIDE_UNDOC_CLASSES tag is set to YES, doxygen will hide all\n> > # undocumented classes that are normally visible in the class hierarchy.\n> > If set> \n> > diff --git a/src/libcamera/media_device.cpp\n> > b/src/libcamera/media_device.cpp index b86d0c4..48aa805 100644\n> > --- a/src/libcamera/media_device.cpp\n> > +++ b/src/libcamera/media_device.cpp\n> > @@ -641,6 +641,28 @@ bool MediaDevice::populateLinks(const struct\n> > media_v2_topology &topology)\n> > \treturn true;\n> > }\n> > \n> > +/**\n> > + * \\brief Apply \\a flags to a link between two pads\n> > + * \\param source The source pad\n> > + * \\param sink The sink pad\n> > + * \\param flags The link flags\n> > + *\n> > + * Implements 'raw' link handling, as this functions applies \\a flags,\n> > + * (whose only accepted values are the ones defined by the Media\n> > Controller + * APIs in MEDIA_LNK_FL_* macros) to a link between two pads.\n> > No correctness + * checks on the link existence and validity of \\a flags\n> > is performed. The + * function assumes the \\a source and \\a sink pads are\n> > connected, and the + * supplied \\a flags applies to the link (ie.\n> > immutable links cannot be + * disabled).\n> > + *\n> > + * This function wraps the raw MEDIA_IOC_SETUP_LINK ioctl, and shouldn't\n> > be + * called directly by any other class or method but the\n> > + * MediaLink::setup(bool enable) one, declared as friend for this reason.\n> \n> I don't love this, make setLink() public :-)\n\nI think there's value in avoiding potential problems, so keeping setLink() \nprivate with a friend is fine with me, but we need to find a solution for the \ndoxygen issue pointed out above. We would need a HIDE_UNDOC_PRIVATE_MEMBERS \ndirective, which doesn't seem to be available :-( If we have to choose between \nnot generating documentation for private members and dropping warnings for all \nundocumented members, the former is unfortunately the only viable option.\n\n> > + *\n> > + * \\sa MediaLink::setup(bool enable)\n> > + *\n> > + * \\return 0 for success, negative error number otherwise\n> > + */\n> > int MediaDevice::setLink(const MediaPad *source, const MediaPad *sink,\n> > \t\t\t unsigned int flags)\n> > {","headers":{"Return-Path":"","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 1AB85600CC\n\tfor ;\n\tMon, 7 Jan 2019 22:57:09 +0100 (CET)","from avalon.localnet (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8564FE4E;\n\tMon, 7 Jan 2019 22:57:08 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1546898228;\n\tbh=4FuT4DpeiZ2hWdhLYzIPUh6+wZ/q7Y05rlKGh9feTEU=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=i5Ykae98SAtZqPiFPgamMWTgkfLW3FPodKDDN14FMgApDtaCsDKnOhEutUXjW8ofg\n\t1emom226IiPnhxYMBrg56Ee/s3a62NvgIJC7BJ0KngggdI+V26cyrWEIxz8DbZi2ci\n\tyPQtMX3CmKyBZtR4VePUjIDjAwhKCo6XvC/qWO0Y=","From":"Laurent Pinchart ","To":"libcamera-devel@lists.libcamera.org","Date":"Mon, 07 Jan 2019 23:58:16 +0200","Message-ID":"<6022267.4GnW3kY2sG@avalon>","Organization":"Ideas on Board Oy","In-Reply-To":"<20190104164730.GO22790@bigcity.dyn.berto.se>","References":"<20190103173859.22624-1-jacopo@jmondi.org>\n\t<20190103173859.22624-5-jacopo@jmondi.org>\n\t<20190104164730.GO22790@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","Content-Type":"text/plain; charset=\"iso-8859-1\"","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: Document private\n\tmembers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 07 Jan 2019 21:57:09 -0000"}},{"id":239,"web_url":"https://patchwork.libcamera.org/comment/239/","msgid":"<3822710.v9H9PeRcav@avalon>","date":"2019-01-07T22:03:32","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: Document private\n\tmembers","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 Thursday, 3 January 2019 19:38:58 EET Jacopo Mondi wrote:\n> The newly added MediaDevice::setLink() function is defined as private,\n> but it is worth being documented, as it is called from the friend method\n> MediaLink::setup().\n> \n> In the library code base, several private methods and fields are\n> documented, but do not show up in the generated documentation output.\n> Change doxygen settings to output processed documentation for the\n> private fields and methods, when proper doxygen comments are applied to\n> them (and do not complain if private members are not documented at all).\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> Documentation/Doxyfile.in | 4 ++--\n> src/libcamera/media_device.cpp | 22 ++++++++++++++++++++++\n> 2 files changed, 24 insertions(+), 2 deletions(-)\n> \n> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> index b1a70d3..16dcccd 100644\n> --- a/Documentation/Doxyfile.in\n> +++ b/Documentation/Doxyfile.in\n> @@ -442,7 +442,7 @@ EXTRACT_ALL = NO\n> # be included in the documentation.\n> # The default value is: NO.\n> \n> -EXTRACT_PRIVATE = NO\n> +EXTRACT_PRIVATE = YES\n> \n> # If the EXTRACT_PACKAGE tag is set to YES, all members with package or\n> internal # scope will be included in the documentation.\n> @@ -487,7 +487,7 @@ EXTRACT_ANON_NSPACES = NO\n> # section is generated. This option has no effect if EXTRACT_ALL is\n> enabled. # The default value is: NO.\n> \n> -HIDE_UNDOC_MEMBERS = NO\n> +HIDE_UNDOC_MEMBERS = YES\n> \n> # If the HIDE_UNDOC_CLASSES tag is set to YES, doxygen will hide all\n> # undocumented classes that are normally visible in the class hierarchy. If\n> set diff --git a/src/libcamera/media_device.cpp\n> b/src/libcamera/media_device.cpp index b86d0c4..48aa805 100644\n> --- a/src/libcamera/media_device.cpp\n> +++ b/src/libcamera/media_device.cpp\n> @@ -641,6 +641,28 @@ bool MediaDevice::populateLinks(const struct\n> media_v2_topology &topology) return true;\n> }\n> \n> +/**\n> + * \\brief Apply \\a flags to a link between two pads\n> + * \\param source The source pad\n> + * \\param sink The sink pad\n\nHow about passing the link pointer instead ?\n\n> + * \\param flags The link flags\n> + *\n> + * Implements 'raw' link handling, as this functions applies \\a flags,\n> + * (whose only accepted values are the ones defined by the Media Controller\n> + * APIs in MEDIA_LNK_FL_* macros) to a link between two pads. No\n> correctness + * checks on the link existence and validity of \\a flags is\n> performed. The + * function assumes the \\a source and \\a sink pads are\n> connected, and the + * supplied \\a flags applies to the link (ie. immutable\n> links cannot be + * disabled).\n\n\"This function applies the link \\a flags (as defined by the MEDIA_LNK_FL_* \nmacros from the Media Controller API) to the given \\a link. It implements low-\nlevel link setup as it performs no checks on the validity of the \\a flags, and \nassumes that the supplied \\a flags are valid for the link (e.g. immutable \nlinks cannot be disabled).\"\n\n> + *\n> + * This function wraps the raw MEDIA_IOC_SETUP_LINK ioctl, and shouldn't be\n> + * called directly by any other class or method but the\n> + * MediaLink::setup(bool enable) one, declared as friend for this reason.\n> + *\n> + * \\sa MediaLink::setup(bool enable)\n> + *\n> + * \\return 0 for success, negative error number otherwise\n> + */\n> int MediaDevice::setLink(const MediaPad *source, const MediaPad *sink,\n> \t\t\t unsigned int flags)\n\nShould this be named setupLink() to refer to the MediaController terminology ?\n\n> {","headers":{"Return-Path":"","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 384A8600CC\n\tfor ;\n\tMon, 7 Jan 2019 23:02:25 +0100 (CET)","from avalon.localnet (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B59F0E4E;\n\tMon, 7 Jan 2019 23:02:24 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1546898544;\n\tbh=HGQyprQv4GRs20jQq+2rDMt3Stk0khG8xlT5S1/VXag=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=XDHjageB865RbydNGaG53OWJvsrUBTUvVUAKUEtSbrLd2IzZsbToe+AjoEIGD22oI\n\t0KjKkyniF2QNvtNyhl5SkDT9LbBb9pdJcepKPpht0z9fBre1PEbn67nHZV5OhwSgPR\n\tSROCGMaHUs3fZAWdfp+WL5wcwgRCRDs5Nq5H1oIE=","From":"Laurent Pinchart ","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 08 Jan 2019 00:03:32 +0200","Message-ID":"<3822710.v9H9PeRcav@avalon>","Organization":"Ideas on Board Oy","In-Reply-To":"<20190103173859.22624-5-jacopo@jmondi.org>","References":"<20190103173859.22624-1-jacopo@jmondi.org>\n\t<20190103173859.22624-5-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Transfer-Encoding":"7Bit","Content-Type":"text/plain; charset=\"us-ascii\"","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: Document private\n\tmembers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 07 Jan 2019 22:02:25 -0000"}}]