RSDK-9726 - improve modulegen help text and deprecate resource-type flag

.github/workflows/test-module-generation.yml

@@ -13,29 +13,29 @@ jobs:
         language: ["python", "go"]
         resource:
           [
-            { subtype: "arm", type: "component" },
-            { subtype: "audio_input", type: "component" },
-            { subtype: "base", type: "component" },
-            { subtype: "board", type: "component" },
-            { subtype: "camera", type: "component" },
-            { subtype: "encoder", type: "component" },
-            { subtype: "gantry", type: "component" },
-            { subtype: "generic", type: "component" },
-            { subtype: "gripper", type: "component" },
-            { subtype: "input", type: "component" },
-            { subtype: "motor", type: "component" },
-            { subtype: "movement_sensor", type: "component" },
-            { subtype: "pose_tracker", type: "component" },
-            { subtype: "power_sensor", type: "component" },
-            { subtype: "sensor", type: "component" },
-            { subtype: "servo", type: "component" },
+            { subtype: "arm" },
+            { subtype: "audio_input" },
+            { subtype: "base" },
+            { subtype: "board" },
+            { subtype: "camera" },
+            { subtype: "encoder" },
+            { subtype: "gantry" },
+            { subtype: "generic_component" },
+            { subtype: "gripper" },
+            { subtype: "input" },
+            { subtype: "motor" },
+            { subtype: "movement_sensor" },
+            { subtype: "pose_tracker" },
+            { subtype: "power_sensor" },
+            { subtype: "sensor" },
+            { subtype: "servo" },
 
-            { subtype: "generic", type: "service" },
-            { subtype: "mlmodel", type: "service" },
-            { subtype: "motion", type: "service" },
-            { subtype: "navigation", type: "service" },
-            { subtype: "slam", type: "service" },
-            { subtype: "vision", type: "service" },
+            { subtype: "generic-service" },
+            { subtype: "mlmodel" },
+            { subtype: "motion" },
+            { subtype: "navigation" },
+            { subtype: "slam" },
+            { subtype: "vision" },
           ]
     steps:
       - name: Checkout Code
@@ -53,7 +53,6 @@ jobs:
           --language "${{ matrix.language }}" \
           --public-namespace "my-org" \
           --resource-subtype "${{ matrix.resource.subtype }}" \
-          --resource-type "${{ matrix.resource.type }}" \
           --model-name "model-name" \
           --dry-run
 

cli/app.go

@@ -2195,31 +2195,37 @@ After creation, use 'viam module update' to push your new module to app.viam.com
 					Flags: []cli.Flag{
 						&cli.StringFlag{
 							Name:  generalFlagName,
-							Usage: "name to use for module",
+							Usage: "name to use for module. for example, a module that contains sensor implementations might be named 'sensors'",
 						},
 						&cli.StringFlag{
 							Name:  moduleFlagLanguage,
-							Usage: "language to use for module",
+							Usage: formatAcceptedValues("language to use for module", supportedModuleGenLanguages...),
 						},
 						&cli.BoolFlag{
 							Name:  moduleFlagIsPublic,
 							Usage: "set module to public",
 						},
 						&cli.StringFlag{
-							Name:  moduleFlagPublicNamespace,
-							Usage: "namespace or organization ID of module",
+							Name: moduleFlagPublicNamespace,
+							Usage: "namespace or organization ID of module. " +
+								"must be either a valid organization ID, or a namespace that exists within a user organization",
 						},
 						&cli.StringFlag{
-							Name:  generalFlagResourceSubtype,
-							Usage: "resource subtype to use in module",
+							Name: generalFlagResourceSubtype,
+							Usage: "resource subtype to use in module, for example arm, camera, or motion. see " +
+								"https://docs.viam.com/dev/reference/glossary/#term-subtype for more details",
 						},
+						// This is unnecessary and creates a gotcha for users. Kept here
+						// because it's technically breaking to remove it, but it's hidden
+						// and serves no purpose.
 						&cli.StringFlag{
-							Name:  moduleFlagResourceType,
-							Usage: "resource type to use in module",
+							Name:   moduleFlagResourceType,
+							Hidden: true,
 						},
 						&cli.StringFlag{
-							Name:  moduleFlagModelName,
-							Usage: "resource model name to use in module",
+							Name: moduleFlagModelName,
+							Usage: "name for the particular resource subtype implementation." +
+								" for example, a sensor model that detects moisture might be named 'moisture'",
 						},
 						&cli.BoolFlag{
 							Name:  moduleFlagEnableCloud,

cli/module_generate.go

@@ -41,6 +41,8 @@ const (
 	golang         = "go"
 )
 
+var supportedModuleGenLanguages = []string{python, golang}
+
 var (
 	scriptsPath   = filepath.Join(basePath, "scripts")
 	templatesPath = filepath.Join(basePath, "_templates")
@@ -51,7 +53,6 @@ type generateModuleArgs struct {
 	Language        string
 	Public          bool
 	PublicNamespace string
-	ResourceType    string
 	ResourceSubtype string
 	ModelName       string
 	EnableCloud     bool
@@ -77,14 +78,13 @@ func (c *viamClient) generateModuleAction(cCtx *cli.Context, args generateModule
 		Language:         args.Language,
 		IsPublic:         args.Public,
 		Namespace:        args.PublicNamespace,
-		Resource:         args.ResourceSubtype + " " + args.ResourceType,
-		ResourceType:     args.ResourceType,
 		ResourceSubtype:  args.ResourceSubtype,
 		ModelName:        args.ModelName,
 		EnableCloudBuild: args.EnableCloud,
 		RegisterOnApp:    args.Register,
 	}
-	if err := newModule.CheckResource(); err != nil {
+
+	if err := newModule.CheckResourceAndSetType(); err != nil {
 		return err
 	}
 
@@ -205,7 +205,16 @@ func promptUser(module *modulegen.ModuleInputs) error {
 				words[i] = titleCaser.String(word)
 			}
 		}
-		resourceOptions = append(resourceOptions, huh.NewOption(strings.Join(words, " "), resource))
+		// we differentiate generic-service and generic-component in `modulegen.Resources`
+		// but they still have the type listed. This carveout prevents the user prompt from
+		// suggesting `Generic Component Component` or `Generic Service Service` as an option
+		var resType string
+		if words[0] == "Generic" {
+			resType = strings.Join(words[:2], " ")
+		} else {
+			resType = strings.Join(words, " ")
+		}
+		resourceOptions = append(resourceOptions, huh.NewOption(resType, resource))
 	}
 	form := huh.NewForm(
 		huh.NewGroup(
@@ -335,12 +344,17 @@ func catchResolveOrgErr(cCtx *cli.Context, c *viamClient, newModule *modulegen.M
 func populateAdditionalInfo(newModule *modulegen.ModuleInputs) {
 	newModule.GeneratedOn = time.Now().UTC()
 	newModule.GeneratorVersion = version
+	// TODO(RSDK-9727) - this is a bit inefficient because `newModule.Resource` is set above in
+	// `generateModuleAction` based on `ResourceType` and `ResourceSubtype`, which are then
+	// overwritten based on `newModule.Resource`! Unfortunately fixing this is slightly complicated
+	// due to cases where a user didn't pass a `ResourceSubtype`, and so it was set in the `promptUser`
+	// call. We should look into simplifying though, such that all these values are only ever set once.
 	newModule.ResourceSubtype = strings.Split(newModule.Resource, " ")[0]
 	newModule.ResourceType = strings.Split(newModule.Resource, " ")[1]
 
 	titleCaser := cases.Title(language.Und)
 	replacer := strings.NewReplacer("_", " ", "-", " ")
-	spaceReplacer := strings.NewReplacer(" ", "", "_", "", "-", "")
+	spaceReplacer := modulegen.SpaceReplacer
 	newModule.ModulePascal = spaceReplacer.Replace(titleCaser.String(replacer.Replace(newModule.ModuleName)))
 	newModule.ModuleCamel = strings.ToLower(string(newModule.ModulePascal[0])) + newModule.ModulePascal[1:]
 	newModule.ModuleLowercase = strings.ToLower(newModule.ModulePascal)

cli/module_generate/modulegen/inputs.go

@@ -2,7 +2,9 @@
 package modulegen
 
 import (
+	"errors"
 	"fmt"
+	"strings"
 	"time"
 )
 
@@ -46,7 +48,7 @@ var Resources = []string{
 	"camera component",
 	"encoder component",
 	"gantry component",
-	"generic component",
+	"generic_component component",
 	"gripper component",
 	"input component",
 	"motor component",
@@ -55,7 +57,7 @@ var Resources = []string{
 	"power_sensor component",
 	"sensor component",
 	"servo component",
-	"generic service",
+	"generic_service service",
 	"mlmodel service",
 	"motion service",
 	"navigation service",
@@ -75,7 +77,7 @@ type GoModuleTmpl struct {
 // HasEmptyInput checks to see if any required inputs were not filled in.
 func (inputs *ModuleInputs) HasEmptyInput() bool {
 	requiredInputs := []string{
-		inputs.ModuleName, inputs.Language, inputs.Namespace, inputs.ResourceType, inputs.ResourceSubtype, inputs.ModelName,
+		inputs.ModuleName, inputs.Language, inputs.Namespace, inputs.ResourceSubtype, inputs.ModelName,
 	}
 	for _, input := range requiredInputs {
 		if input == "" {
@@ -85,15 +87,40 @@ func (inputs *ModuleInputs) HasEmptyInput() bool {
 	return false
 }
 
-// CheckResource checks if the given resource is valid.
-func (inputs *ModuleInputs) CheckResource() error {
-	if inputs.ResourceSubtype == "" || inputs.ResourceType == "" {
+// SpaceReplacer removes spaces, dashes, and underscores from a string.
+var SpaceReplacer = strings.NewReplacer(" ", "", "_", "", "-", "")
+
+// CheckResourceAndSetType checks if the given resource subtype is valid, and sets the corresponding resource type if so.
+func (inputs *ModuleInputs) CheckResourceAndSetType() error {
+	if inputs.ResourceSubtype == "" {
 		return nil
 	}
+	if inputs.ResourceSubtype == "generic" {
+		return errors.New(
+			"resource subtype 'generic' cannot be differentiated; please specify either 'generic-service' or 'generic-component'")
+	}
 	for _, resource := range Resources {
-		if inputs.Resource == resource {
+		splitResource := strings.Split(resource, " ")
+		if len(splitResource) != 2 {
+			return errors.New("resource not formatted correctly in code base; this shouldn't happen. please consider filing a ticket")
+		}
+		subtype := splitResource[0]
+		// make sure we support subtypes that are passed with different spacers (e.g.,
+		// "power sensor", "power-sensor", "power_sensor", "powerSensor")
+		if strings.ToLower(SpaceReplacer.Replace(inputs.ResourceSubtype)) == SpaceReplacer.Replace(subtype) {
+			// we need users to specify if a generic is a component or service, but we want to
+			// internally simplify the subtype back down to `generic`
+			if subtype == "generic_component" || subtype == "generic_service" {
+				inputs.ResourceSubtype = "generic"
+			} else {
+				// use the canonically correct subtype formatting so we don't have to continue
+				// supporting all variations of it everywhere in the codebase
+				inputs.ResourceSubtype = subtype
+			}
+			inputs.ResourceType = splitResource[1]
+			inputs.Resource = inputs.ResourceSubtype + " " + inputs.ResourceType
 			return nil
 		}
 	}
-	return fmt.Errorf("given resource '%s' does not exist", inputs.Resource)
+	return fmt.Errorf("given resource subtype '%s' does not exist", inputs.ResourceSubtype)
 }