DOCS-514: Add C++ MR create examples #2277

andf-viam · 2023-11-29T17:39:12Z

Add C++ example code to Create docs for MR.

Drew from complex example in C++ SDK, as well as module-example-cpp repo.
Fixed instances of rdk:service:base, which should be rdk:component:base instead, per Rand.
(out of scope): Fixed instances of rdk:components:* elsewhere in the docs, these should use singular component.

CODE SAMPLES:

Thanks so much Benji and Ethan for the help! Compiles and appears to run without errors.

QUESTIONS:

You've answered all my questions, thank you!

NOTES:

Changing 'client' language outside of C++ scope on this page to be handled separately in DOCS - 1527, per discussion thread. LMK if there are other opportunities to fix C++-specific language that I missed though!

viambot · 2023-11-29T17:40:19Z

Overall readability score: 56.56 (🟢 +0)

File	Readability
_index.md	51.87 (🟢 +0.3)
custom-base-dog.md	70.39 (🟢 +0)

View detailed metrics

🟢 - Shows an increase in readability
🔴 - Shows a decrease in readability

File	Readability	FRE	GF	ARI	CLI	DCRS
_index.md	51.87	32.73	10.32	16.3	13.46	6.18
	🟢 +0.3	🔴 -0.51	🔴 -0.02	🔴 -0.1	🟢 +0.06	🟢 +0.15
custom-base-dog.md	70.39	54.22	9.05	10.6	10.15	6.23
	🟢 +0	🟢 +0	🟢 +0	🟢 +0	🟢 +0	🟢 +0

Averages:

	Readability	FRE	GF	ARI	CLI	DCRS
Average	56.56	48.52	10.49	12.92	11.91	7.65
	🟢 +0	🟢 +0	🟢 +0	🟢 +0	🟢 +0	🟢 +0

View metric targets

Metric	Range	Ideal score
Flesch Reading Ease	100 (very easy read) to 0 (extremely difficult read)	60
Gunning Fog	6 (very easy read) to 17 (extremely difficult read)	8 or less
Auto. Read. Index	6 (very easy read) to 14 (extremely difficult read)	8 or less
Coleman Liau Index	6 (very easy read) to 17 (extremely difficult read)	8 or less
Dale-Chall Readability	4.9 (very easy read) to 9.9 (extremely difficult read)	6.9 or less

andf-viam · 2023-11-29T21:02:41Z

docs/registry/create/_index.md

+#include <boost/log/trivial.hpp>
+
+// Within your main() program, before logging anything, accept command line arguments to control log severity:
+set_logger_severity_from_args(argc, argv);


Q: Or is this only required when performing debug-level logging, and not required otherwise?

I believe this should only be necessary when you want debug logging.

andf-viam · 2023-11-29T21:03:54Z

docs/registry/create/_index.md

+{{% alert title="Important" color="note" %}}
+
+You must define all functions belonging to a built-in resource subtype's API if defining a new model.
+Otherwise, the class will not instantiate.


Is this true for C++, like it is for Python and Go?

If so, is the next line about throw-ing a runtime_error correct? (Found here)

If not, what notice should be here, if any?

This is a good question. What do we mean by a built-in resource subtype's API? More specifically, what is the source of truth for this?

If we look at the header files for a built-in resource for our source of truth, there are definitely APIs that don't need to be re-implemented (see, e.g., resource_registration() and dynamic_api(). What a user does need to define are all the pure virtual methods.

If we look at what the definition of, say, a camera is based on camera.proto, then yes, you need to define all the functions.

On the below point, throwing a runtime_error is a totally valid way of avoiding putting in an actual implementation.

Thank you! I think we mean "camera (et al) API", which does indeed sounds like the hpp file definitions, like camera.hpp.

Ive clarified this blurb with your guidance, specifically around "implement this one but not that one."

npentrel

This looks good to me from a docs perspective - for the cpp files do we need a header file?

npentrel · 2023-12-05T11:25:06Z

Ok - then @andf-viam let's also include the headers here. Based on https://github.com/viamrobotics/module-example-cpp/blob/main/src/wifi.hpp I think this shouldn't be much

stuqdog

I forgot to submit review from a while ago 🤦 sorry!

stuqdog · 2023-11-30T17:13:41Z

docs/registry/create/_index.md

+{{% /tab %}}
+{{% tab name="C++" %}}
+
+To create a new resource model, you need to implement your model's **client** interface in a file called `my_modular_resource.cpp`.


I may be misunderstanding but I don't think this is right. The client code represents a connection to the server to call on a particular instance of a resource. To define a model broadly I think we should be implementing the resource base class, e.g. src/viam/sdk/components/camera/*camera*.hpp

I too may be misunderstanding! I want to try to leave initial mention at cpp because it's wholly possible to write a module using just one cpp file.

But certainly many uses will also include hpp files, so I've added a new sentence on line 76-77 below to mention. LMK if I'm close!

I think with the example code below now showing example hpp code as well, this might be sufficient, but LMK!

stuqdog · 2023-11-30T17:14:21Z

docs/registry/create/_index.md

+
+To ensure the client interface you create returns the expected results, use the appropriate client interface defined in <file>components/\<resource-name\>/client.cpp</file> or <file>services/\<resource-name\>/client.cpp</file> in the [Viam C++ SDK](https://github.com/viamrobotics/viam-cpp-sdk/tree/main/src/viam/sdk) as a reference.
+
+For example, the `base` component client is defined in the [<file>components/base/client.cpp</file>](https://github.com/viamrobotics/viam-cpp-sdk/blob/main/src/viam/sdk/components/base/client.cpp) file.


We should link to the header files (*.hpp) rather than implementation files (*.cpp).

stuqdog · 2023-11-30T17:22:29Z

docs/registry/create/_index.md

@@ -74,14 +87,21 @@ See [Valid APIs to implement in your model](#valid-apis-to-implement-in-your-mod

 Find the subtype API as defined in the relevant <file>components/\<resource-name\>/\<resource-name>\.py</file> or <file>services/\<resource-name\>/<resource-name>.py</file> file in the [Python SDK](https://github.com/viamrobotics/viam-python-sdk).

-For example, the `base` component subtype is defined in [<file>viam-python-sdk/src/viam/components/base/base.py</file>](https://github.com/viamrobotics/viam-python-sdk/blob/main/src/viam/components/base/base.py).
+For example, the `base` component subtype is defined in [<file>components/base/base.py</file>](https://github.com/viamrobotics/viam-python-sdk/blob/main/src/viam/components/base/base.py#L11).


The language component subtype is being used for each of the languages here but I think we mean component API?

stuqdog · 2023-11-30T18:59:10Z

docs/registry/create/_index.md

+The following example module registers a modular resource implementing Viam's built-in [Base API](/build/configure/components/base/#api) (`rdk:component:base`) as a new model, `"MyBase"`.
+
+<details>
+  <summary>Click to view sample code for <file>my_base.cpp</file></summary>


I think we need to include a definition of MyBase, either as a my_base.hpp header file or defined at top of the .cpp file. Right now your code sample is referencing data members of the class (e.g., left_ and right_) that aren't actually defined anywhere that I can see in this doc.

Added a my_base.hpp header file, based on this example!

stuqdog · 2023-11-30T19:55:02Z

docs/registry/create/_index.md

+    Model mybase_model("acme", "demo", "mybase");
+
+    std::shared_ptr<ModelRegistration> mybase_mr = std::make_shared<ModelRegistration>(
+        ResourceType("Base"),


ResourceType is no longer a thing as of this afternoon, so we'll want to remove this line.

stuqdog · 2023-11-30T19:56:38Z

docs/registry/create/_index.md

+#include <boost/log/trivial.hpp>
+
+// Within your main() program, before logging anything, accept command line arguments to control log severity:
+set_logger_severity_from_args(argc, argv);


I believe this should only be necessary when you want debug logging.

stuqdog · 2023-11-30T19:57:35Z

docs/registry/create/_index.md

+   // To log a simple message at debug level:
+   BOOST_LOG_TRIVIAL(debug) << "Starting module with debug level logging";
+   // To log a message that contains a variable at info level:
+   BOOST_LOG_TRIVIAL(info) << "Detected the following variable value: " << my_var;


Note that not all types can be logged in this way, only those that implement <<

andf-viam · 2023-12-15T20:04:39Z

docs/registry/create/_index.md

+
+```cpp {class="line-numbers linkable-line-numbers"}
+#include <boost/log/trivial.hpp>
+```


Per Benji, all std::cout messages are silently forwarded up to the Viam app via gRPC. Happy to add any additional context here if you have any!

I hear this is changing in the near/medium term, just file a ticket when this changes and I'll update!

andf-viam · 2023-12-15T20:06:53Z

@stuqdog @benjirewis Hi folks! Thanks so much for your help, both of you! Here's an update based on Benji's complex module code (but with everything but the mybase impl thrown out). Let me know if I'm closer to the target, or if there's anything you'd like to see adjusted.

Thanks so much both of you for the help and guidance!!

andf-viam · 2023-12-15T22:57:29Z

docs/registry/create/_index.md

+{{% /tab %}}
+{{% tab name="C++" %}}
+
+To create a new resource model, start by referencing the corresponding built-in resource's implementation in the [Viam C++ SDK](https://github.com/viamrobotics/viam-cpp-sdk/tree/main/src/viam/sdk).


Currently, lines 68-73 are basically the same as lines 101-103. This will change when I move on to DOCS - 1527 (point to base.X files not client.X files) after this, where one or the other section will be removed. Keeping here just to match other two languages in the interim. With DOCS - 1527 all three will use similar, shorter, non-replicated copy! 👍

stuqdog

looks good to me! I haven't tested the sample code however, have you tested to make sure it works?

benjirewis · 2023-12-18T16:42:45Z

docs/registry/create/_index.md

+
+std::vector<std::string> MyBase::validate(ResourceConfig cfg) {
+    // Custom validation can be done by specifying a validate function at the
+    // time of resource registration (see complex/main.cpp) like this one.


Suggested change

// time of resource registration (see complex/main.cpp) like this one.

// time of resource registration (see main.cpp) like this one.

benjirewis · 2023-12-18T16:43:37Z

docs/registry/create/_index.md

+</details>
+<br>
+
+Additional example modules are available in the [C++ SDK GitHub repository](https://github.com/viamrobotics/viam-cpp-sdk/tree/main/src/viam/examples/).


Suggested change

Additional example modules are available in the [C++ SDK GitHub repository](https://github.com/viamrobotics/viam-cpp-sdk/tree/main/src/viam/examples/).

Additional example modules are available in the [C++ SDK GitHub repository](https://github.com/viamrobotics/viam-cpp-sdk/tree/main/src/viam/examples/modules).

benjirewis · 2023-12-18T16:45:36Z

docs/registry/create/_index.md

+
+   cd $(dirname $0)
+   # get bundled .so files from this directory
+   export LD_LIBRARY_PATH=${LD_LIBRARY_PATH-}:$PWD


Did you find this line to be necessary for building? I don't really know what it does.

You're right, we're not building any .sos for this example, I'll remove this!

(tested the compiled module using a run.sh without this line, works as expected 👍 )

Ah did you want to remove this export and the comment above?

benjirewis · 2023-12-18T16:47:44Z

docs/registry/create/_index.md

+{{% /tab %}}
+{{% tab name="C++" %}}
+
+Messages sent to `std::cout` in your C++ code are automatically sent to the Viam app over gRPC when a network connection is available.


This is not exactly true, and apologies for my probably misleading explanation. std::cout messages certainly go to STDOUT. The RDK gathers all the STDOUT output from the process and logs it (no gRPC involved). We will soon have a custom C++ logger that logs over gRPC back to the parent when possible and logs to STDOUT otherwise. But, std::cout messages will never be sent back over gRPC, only messages logged through that not-yet-existing logger.

Thanks! Removed mention of gRPC 👍 Happy to circle back to this later and update when you make the upcoming custom logger change!

We technical writers are actually somewhat discouraged from documenting the future; something about leaving room for science fiction writers. 🤷 😂

andf-viam · 2023-12-18T17:50:59Z

looks good to me! I haven't tested the sample code however, have you tested to make sure it works?

It do!

Registered component from module

2023-12-18T12:36:56.358-0500	INFO	robot_server.process.my-module_/opt/andf/my-module/run.sh.StdOut	pexec/managed_process.go:244
\_ [2023-12-18 12:36:56.358657] [0x00000001f25ee080] [info]    Module listening on /var/folders/2p/dy4wkkhs0n10_b9tk1nhn0v80000gn/T/viam-module-904747661/my-module.sock
2023-12-18T12:36:56.359-0500	INFO	robot_server.process.my-module_/opt/andf/my-module/run.sh.StdOut	pexec/managed_process.go:244
\_ [2023-12-18 12:36:56.358925] [0x00000001f25ee080] [info]    Module handles the following API/model pairs:
2023-12-18T12:36:56.359-0500	INFO	robot_server.process.my-module_/opt/andf/my-module/run.sh.StdOut	pexec/managed_process.go:244
\_ API: rdk:component:base
2023-12-18T12:36:56.359-0500	INFO	robot_server.process.my-module_/opt/andf/my-module/run.sh.StdOut	pexec/managed_process.go:244
\_ 	Model: acme:demo:mybase
2023-12-18T12:36:56.359-0500	INFO	robot_server.process.my-module_/opt/andf/my-module/run.sh.StdOut	pexec/managed_process.go:244
\_
2023-12-18T12:36:56.361-0500	INFO	robot_server	modmanager/manager.go:862	registering component from module	{"module":"my-module","API":"rdk:component:base","model":"acme:demo:mybase"}

andf-viam · 2023-12-18T17:56:43Z

@stuqdog @benjirewis Thanks both for the help and feedback! Benji, back to you for a follow-up round following your comments, thank you!

benjirewis

Nice! LGTM % potentially removing that export.

benjirewis · 2023-12-18T21:05:10Z

docs/registry/create/_index.md

+
+   cd $(dirname $0)
+   # get bundled .so files from this directory
+   export LD_LIBRARY_PATH=${LD_LIBRARY_PATH-}:$PWD


Ah did you want to remove this export and the comment above?

andf-viam · 2023-12-18T21:09:18Z

@benjirewis Thanks! Yep, removed from test code but forgot to remove here. Thanks for catching!

viambot · 2023-12-18T21:12:39Z

You can view a rendered version of the docs from this PR at https://docs-test.viam.dev/2277

viambot added the safe to build This pull request is marked safe to build from a trusted zone label Nov 29, 2023

andf-viam marked this pull request as ready for review November 29, 2023 20:34

andf-viam commented Nov 29, 2023

View reviewed changes

npentrel reviewed Dec 2, 2023

View reviewed changes

stuqdog requested changes Dec 5, 2023

View reviewed changes

andf-viam commented Dec 15, 2023

View reviewed changes

andf-viam requested a review from stuqdog December 15, 2023 20:07

andf-viam added 7 commits December 15, 2023 17:00

DOCS-514: Add C++ MR create examples

a7ca794

fix language

b4d012b

fix API triplet OOS

1014f44

tweak code

29fb24d

apply feedback

7b4bb86

fix langauge

2471a9e

add space btwn details

355ac4b

andf-viam force-pushed the DOCS-514-add-cpp-mr-examples branch from 63695e0 to 355ac4b Compare December 15, 2023 22:06

andf-viam added 2 commits December 15, 2023 17:24

fix link

2cecef2

tweak client

ada9861

andf-viam commented Dec 15, 2023

View reviewed changes

stuqdog approved these changes Dec 18, 2023

View reviewed changes

benjirewis requested changes Dec 18, 2023

View reviewed changes

apply feedback

c908049

andf-viam requested a review from benjirewis December 18, 2023 17:56

benjirewis approved these changes Dec 18, 2023

View reviewed changes

remove unneeded line

ae2398b

andf-viam merged commit 08b0220 into viamrobotics:main Dec 18, 2023
11 checks passed

andf-viam deleted the DOCS-514-add-cpp-mr-examples branch December 18, 2023 21:15

npentrel pushed a commit that referenced this pull request Aug 29, 2024

DOCS-514: Add C++ MR create examples (#2277)

9ad74b1


		To ensure the client interface you create returns the expected results, use the appropriate client interface defined in <file>components/\<resource-name\>/client.cpp</file> or <file>services/\<resource-name\>/client.cpp</file> in the [Viam C++ SDK](https://github.com/viamrobotics/viam-cpp-sdk/tree/main/src/viam/sdk) as a reference.

		For example, the `base` component client is defined in the [<file>components/base/client.cpp</file>](https://github.com/viamrobotics/viam-cpp-sdk/blob/main/src/viam/sdk/components/base/client.cpp) file.

	// time of resource registration (see complex/main.cpp) like this one.
	// time of resource registration (see main.cpp) like this one.

	Additional example modules are available in the [C++ SDK GitHub repository](https://github.com/viamrobotics/viam-cpp-sdk/tree/main/src/viam/examples/).
	Additional example modules are available in the [C++ SDK GitHub repository](https://github.com/viamrobotics/viam-cpp-sdk/tree/main/src/viam/examples/modules).

DOCS-514: Add C++ MR create examples #2277

DOCS-514: Add C++ MR create examples #2277

Conversation

andf-viam commented Nov 29, 2023 • edited Loading

viambot commented Nov 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andf-viam Nov 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

npentrel left a comment

Choose a reason for hiding this comment

npentrel commented Dec 5, 2023

stuqdog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andf-viam Dec 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andf-viam commented Dec 15, 2023 • edited Loading

Choose a reason for hiding this comment

stuqdog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andf-viam Dec 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andf-viam Dec 18, 2023 • edited Loading

Choose a reason for hiding this comment

andf-viam commented Dec 18, 2023 • edited Loading

andf-viam commented Dec 18, 2023

benjirewis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andf-viam commented Dec 18, 2023

viambot commented Dec 18, 2023

andf-viam commented Nov 29, 2023 •

edited

Loading

viambot commented Nov 29, 2023 •

edited

Loading

andf-viam Nov 29, 2023 •

edited

Loading

andf-viam Dec 15, 2023 •

edited

Loading

andf-viam commented Dec 15, 2023 •

edited

Loading

andf-viam Dec 18, 2023 •

edited

Loading

andf-viam Dec 18, 2023 •

edited

Loading

andf-viam commented Dec 18, 2023 •

edited

Loading