DOCS-1527,DOCS-1594: Refactor Module create page #2401
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
viambot
added
the
safe to build
|
Overall readability score: 55.83 (🟢 +0.01)
View detailed metrics🟢 - Shows an increase in readability
Averages:
View metric targets
|
andf-viam
force-pushed
the
DOCS-1527-refactor-create-module
branch
9 times, most recently
from
71740a2 to
b6acca3
Compare
andf-viam
force-pushed
the
DOCS-1527-refactor-create-module
branch
from
b6acca3 to
e841a65
Compare
andf-viam
force-pushed
the
DOCS-1527-refactor-create-module
branch
from
e841a65 to
d0f8b25
Compare
npentrel
reviewed
Jan 19, 2024
docs/registry/_index.md
Outdated
|
|
||
| For example: | ||
|
|
||
| - The `rand:yahboom:arm` model and the `rand:yahboom:gripper` model uses the repository name [yahboom](https://github.com/viam-labs/yahboom). |
There was a problem hiding this comment.
Suggested change
| - The `rand:yahboom:arm` model and the `rand:yahboom:gripper` model uses the repository name [yahboom](https://github.com/viam-labs/yahboom). | |
| - The `rand:yahboom:arm` model and the `rand:yahboom:gripper` model use the repository name [yahboom](https://github.com/viam-labs/yahboom). |
docs/registry/_index.md
Outdated
| } | ||
| ``` | ||
|
|
||
| - The `viam-labs:audioout:pygame` model uses the repository name [audioout](https://github.com/viam-labs/audioout) |
There was a problem hiding this comment.
Suggested change
| - The `viam-labs:audioout:pygame` model uses the repository name [audioout](https://github.com/viam-labs/audioout) | |
| - The `viam-labs:audioout:pygame` model uses the repository name [audioout](https://github.com/viam-labs/audioout). |
docs/registry/create/_index.md
Outdated
| ] | ||
| description: "Use the Viam module system to implement modular resources that can be included in any Viam-powered machine." | ||
| description: "Create a module to provide a new modular resource to your Viam machine." |
There was a problem hiding this comment.
Suggested change
| description: "Create a module to provide a new modular resource to your Viam machine." | |
| description: "Create a module to provide a new modular resource to your machine." |
docs/registry/create/_index.md
Outdated
| aliases: | ||
| - "/extend/modular-resources/create/" | ||
| - "/modular-resources/create/" | ||
| no_list: true | ||
| --- | ||
|
|
||
| Viam provides built-in support for a variety of different {{< glossary_tooltip term_id="component" text="components" >}} and {{< glossary_tooltip term_id="service" text="services" >}}, but you can also add support for unsupported resources by creating a module. | ||
| A _module_ provides one or more {{< glossary_tooltip term_id="modular-resource" text="modular resources" >}}, which add support for {{< glossary_tooltip term_id="resource" text="resource" >}} {{< glossary_tooltip term_id="type" text="types" >}} or {{< glossary_tooltip term_id="model" text="models" >}} that are not built into Viam. | ||
| Viam provides built-in support for a variety of different {{< glossary_tooltip term_id="component" text="components" >}} and {{< glossary_tooltip term_id="service" text="services" >}} -- collectively called {{< glossary_tooltip term_id="resource" text="resources" >}} -- but you might want to use Viam with a new component type or service model that isn't currently supported. |
There was a problem hiding this comment.
I'm not a huge fan of the interjecting additional information with dashes thing. I think that's generally frowned upon. Maybe we can split it into 2 sentences instead?
docs/registry/create/_index.md
Outdated
| Viam provides built-in support for a variety of different {{< glossary_tooltip term_id="component" text="components" >}} and {{< glossary_tooltip term_id="service" text="services" >}}, but you can also add support for unsupported resources by creating a module. | ||
| A _module_ provides one or more {{< glossary_tooltip term_id="modular-resource" text="modular resources" >}}, which add support for {{< glossary_tooltip term_id="resource" text="resource" >}} {{< glossary_tooltip term_id="type" text="types" >}} or {{< glossary_tooltip term_id="model" text="models" >}} that are not built into Viam. | ||
| Viam provides built-in support for a variety of different {{< glossary_tooltip term_id="component" text="components" >}} and {{< glossary_tooltip term_id="service" text="services" >}} -- collectively called {{< glossary_tooltip term_id="resource" text="resources" >}} -- but you might want to use Viam with a new component type or service model that isn't currently supported. | ||
| To extend support to these resources, you can create a {{< glossary_tooltip term_id="module" text="module" >}}. |
There was a problem hiding this comment.
I don't know what this sentence is supposed to mean. Maybe?
Suggested change
| To extend support to these resources, you can create a {{< glossary_tooltip term_id="module" text="module" >}}. | |
| If the existing resources do not support your hardware or service requirements, you can create a {{< glossary_tooltip term_id="module" text="module" >}}. |
There was a problem hiding this comment.
Trying something different, LMK what you think.
docs/registry/create/_index.md
Outdated
|
|
||
| Find the subtype API as defined in the relevant <file>components/\<resource-name\>/\<resource-name>\.hpp</file> or <file>services/\<resource-name\>/<resource-name>.hpp</file> implementation (header) file in the [C++ SDK](https://github.com/viamrobotics/viam-cpp-sdk/). | ||
| {{% alert title=Note color="note" %}} | ||
| If you want to write a module to extend support to a new type of component that is relatively unique, and doesn't readily correspond to an existing [built-in component API](/build/program/apis/#component-apis), you may want to consider using the [generic API](/components/generic/) with your module instead of extending an existing API. |
There was a problem hiding this comment.
Suggested change
| If you want to write a module to extend support to a new type of component that is relatively unique, and doesn't readily correspond to an existing [built-in component API](/build/program/apis/#component-apis), you may want to consider using the [generic API](/components/generic/) with your module instead of extending an existing API. | |
| If you want to write a module to extend support to a new type of component that is relatively unique, and doesn't readily correspond to an existing [built-in component API](/build/program/apis/#component-apis), consider using the [generic API](/components/generic/) with your module instead of extending an existing API. |
docs/registry/create/_index.md
Outdated
|
|
||
| Use the naming schema: `namespace:repo-name:name`. | ||
| For example, if you want to add support for a new [base component](/components/base/) to Viam, you would start by looking at the built-in base component class. |
There was a problem hiding this comment.
Grammar: If sentences that use conditional clauses must use past tense (simple past or past perfect) in the if-clause of the sentence
docs/registry/create/_index.md
Outdated
|
|
||
| Create a folder for your module and save your code as a file named <file>my_modular_resource.go</file> inside. | ||
| For example, if you want to add support for a new [base component](/components/base/) to Viam, you would start by looking at the built-in base component package. |
|
Hi @mestcihazal and @HipsterBrown -- I've refactored our module creation documentation based on yours and others' feedback -- I'd love a quick read-through from one of you to confirm that this addressed your points! Happy to hear any feedback, and make changes as needed! Thanks so much! And thanks so much, both of you, for the great feedback that spurred this effort on! |
HipsterBrown
approved these changes
Jan 22, 2024
|
|
||
| {{< tabs >}} | ||
| {{% tab name="Python" %}} | ||
| While you can certainly combine the resource model definition and the main program code into a single file if desired (for example, a single `main.py` program that includes both the model definition and the `main()` program that uses it), this guide will use separate files for each. |
There was a problem hiding this comment.
this guide will use separate files for each
Do we have a reason for this that we can share? Is it for clarity or separation of concerns?
There was a problem hiding this comment.
Theory: The latter, great insight. My thinking:
- Keeping a clear path forward from concept to concept, as much as we can here.
- While some of our most simple examples across our repos do use one file for both 1 and 2, all of the more complicated ones use two. Starting with this file structure from the beginning might set users up for success when they later transition to module design that accomplishes more complicated tasks, like a component and a service working together.
- This also helps us to demonstrate including other files and referencing the methods/functions contained within them.
Practice: But, mostly, at the end of the day: our existing sample code for all three languages was split in this fashion. Refactoring to a single file for each would be much work, and for minimal gain.
stuqdog
reviewed
Jan 24, 2024
docs/registry/create/_index.md
Outdated
Comment on lines
553
to
554
| | [components/base/base.cpp](https://github.com/viamrobotics/viam-cpp-sdk/blob/main/src/viam/sdk/components/base/base.cpp) | Defines the specific functionality of the built-in `Base` class. | | ||
| | [components/base/base.hpp](https://github.com/viamrobotics/viam-cpp-sdk/blob/main/src/viam/sdk/components/base/base.hpp) | Defines the implementation of functionality defined in `base.cpp`, which includes the declaration of several built-in functions such as `move_straight()`. | |
There was a problem hiding this comment.
Suggested change
| | [components/base/base.cpp](https://github.com/viamrobotics/viam-cpp-sdk/blob/main/src/viam/sdk/components/base/base.cpp) | Defines the specific functionality of the built-in `Base` class. | | |
| | [components/base/base.hpp](https://github.com/viamrobotics/viam-cpp-sdk/blob/main/src/viam/sdk/components/base/base.hpp) | Defines the implementation of functionality defined in `base.cpp`, which includes the declaration of several built-in functions such as `move_straight()`. | | |
| | [components/base/base.hpp](https://github.com/viamrobotics/viam-cpp-sdk/blob/main/src/viam/sdk/components/base/base.hpp) | Defines the API of the built-in `Base` class, which includes the declaration of several purely virtual built-in functions such as `move_straight()`. | | |
| | [components/base/base.cpp](https://github.com/viamrobotics/viam-cpp-sdk/blob/main/src/viam/sdk/components/base/base.cpp) | Provides implementations of the non-purely virtual functionality defined in `base.hpp`. | |
docs/registry/create/_index.md
Outdated
|
|
||
| {{% alert title="Tip" color="tip" %}} | ||
| You can view the other built-in component classes in similar fashion. | ||
| For example, the `Camera` class is defined in [camera.cpp](https://github.com/viamrobotics/viam-cpp-sdk/blob/main/src/viam/sdk/components/camera/camera.cpp) and its functions declared in [camera.hpp](https://github.com/viamrobotics/viam-cpp-sdk/blob/main/src/viam/sdk/components/camera/camera.hpp), while the `Sensor` class is defined in [sensor.cpp](https://github.com/viamrobotics/viam-cpp-sdk/blob/main/src/viam/sdk/components/sensor/sensor.cpp) and its functions declared in [sensor.hpp](https://github.com/viamrobotics/viam-cpp-sdk/blob/main/src/viam/sdk/components/sensor/sensor.hpp). |
There was a problem hiding this comment.
(minor) I think it would help to be slightly more explicit about the fact that resource base classes are all abstract, and the resource-y methods (stop(), get_image(), etc.) are not defined in, e.g., camera.cpp.
|
You can view a rendered version of the docs from this PR at https://docs-test.viam.dev/2401 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor Create a Module page based on user feedback from several sources:
/registry/overview page for now./registry/overview page for now.Feedback addressed in this PR:
go.viam.com/rdk/examples/customresources/models/mybaseto better reflect what we're doing here. Thanks!go.modandgo.sumwith some minimal context. They are not part of module (Viam-module not Go-module) development directly, but will be seen by those inspecting other, published Go Viam modules (Go Viam!!). Good idea to include, thanks!Entry point (main program) filebecause users might be looking for either: Literalmain.cppprogram file withmain()program (it's in the name) vs. entry point (for the module, from perspective ofviam-server), common phrasing for, say, NPM. Subsequent references within-same-section use shortenedentry point file. Open to feedback if both is too much, ofc.Questions:
TODO: