Tech debt (#52)

* remove duplicate type checking

* Fix validation error name

* Add docs for signal specs

README.md

@@ -11,6 +11,11 @@ Welcome to the **Model Garage**, a Golang toolkit for managing and working with
 1. **Model Creation**: Create models from vspec CSV schemas, represented as Go structs.
 2. **JSON Conversion**: Easily convert JSON data for your models with automatically generated and customizable conversion functions.
 
+## Signal Definitions
+
+Signal definitions can be found in the [spec package](./pkg/schema/spec/spec.md).
+This package is the source of truth for signals used for DIMO Data.
+
 ## Migrations
 
 To create a new migration, run the following command:
@@ -36,29 +41,7 @@ go run github.com/DIMO-Network/model-garage/cmd/codegen -output=pkg/vss  -genera
 
 #### Generation Info
 
-The Model generation is handled by packages in `internal/codegen`. They are responsible for creating Go structs, Clickhouse tables, and conversion functions from the vspec CSV schema and definitions file. definitions file is a YAML file that specifies the conversions for each field in the vspec schema. The conversion functions are meant to be overridden with custom logic as needed. When generation is re-run, the conversion functions are not overwritten. Below is an example of a definitions file:
-
-```yaml
-# vspecName: The name of the VSpec field in the VSS schema
-# required
-- vspecName: DIMO.DefinitionID
-  # conversion: The mapping of the original data to the VSpec field
-  conversion:
-    # originalName: The name of the field in the original data
-    # required
-    originalName: data.definitionID
-
-    # originalType: The data type of the field in the original data
-    originalType: string
-
-    # isArray: Whether the original field is an array or not
-    isArray: false
-
-  # requiredPrivileges: The list of privileges required to access the field
-  # required
-  requiredPrivileges:
-    - VEHICLE_NON_LOCATION_DATA
-```
+The Model generation is handled by packages in `internal/codegen`. They are responsible for creating Go structs, Clickhouse tables, and conversion functions from the vspec CSV schema and definitions file. definitions file is a YAML file that specifies the conversions for each field in the vspec schema. The conversion functions are meant to be overridden with custom logic as needed. When generation is re-run, the conversion functions are not overwritten.
 
 ##### Generation Process
 

pkg/schema/signal.go

@@ -3,6 +3,7 @@ package schema
 
 import (
 	"regexp"
+	"slices"
 	"strings"
 	"unicode"
 )
@@ -19,7 +20,11 @@ const (
 	colLen        = 8
 )
 
-var nonAlphaNum = regexp.MustCompile(`[^a-zA-Z0-9]+`)
+var (
+	nonAlphaNum = regexp.MustCompile(`[^a-zA-Z0-9]+`)
+
+	numberTypes = []string{"uint8", "int8", "uint16", "int16", "uint32", "int32", "uint64", "int64", "float", "double", "boolean"}
+)
 
 // SignalInfo holds information about a signal that is accessed during template execution.
 // This information comes from the combinations of the spec and definition files.
@@ -184,21 +189,16 @@ func splitAndSantizeName(name string) (string, string) {
 
 // goTypeFromVSPEC converts vspec type to golang types.
 func goTypeFromVSPEC(baseType string) string {
-	switch baseType {
-	case "uint8", "int8", "uint16", "int16", "uint32", "int32", "uint64", "int64", "float", "double", "boolean":
+	if slices.Contains(numberTypes, baseType) {
 		return "float64"
-	default:
-		return "string"
 	}
+	return "string"
 }
 
 // gqlTypeFromVSPEC converts vspec type to graphql types.
 func gqlTypeFromVSPEC(baseType string) string {
-	switch baseType {
-	// TODO(elffjs): Unify the lists between here and goTypeFromVSPEC?
-	case "uint8", "int8", "uint16", "int16", "uint32", "int32", "uint64", "int64", "float", "double", "boolean":
+	if slices.Contains(numberTypes, baseType) {
 		return "Float"
-	default:
-		return "String"
 	}
+	return "String"
 }

pkg/schema/spec/spec.md

@@ -0,0 +1,39 @@
+# `definitions.yaml`
+
+The [definitions.yaml](./definitions.yaml) file maps vehicle signals to the VSS schema and specifies how to convert and interpret these signals. For a detailed explanation of YAML files, you can refer to the [YAML documentation](https://yaml.org/spec/1.2/spec.html).
+
+## vspecName
+
+The `vspecName` field defines the VSS (Vehicle Signal Specification) name of the singal. The name must be definied in the CSV ouptut of the VSS specification. VSS is a standardized schema defined in [vspec](https://covesa.github.io/vehicle_signal_specification/). This schema is used for vehicle data, and DIMO has its own [fork of the VSS](https://github.com/DIMO-Network/VSS) definitions tailored to our specific needs. This fork includes additional fields and modifications relevant to DIMO's data model.
+
+## conversions
+
+The `conversions` section maps fields from the original data to the VSpec field. Each `conversion` entry specifies:
+
+- **`originalName`**: The name of the field in the original data.
+- **`originalType`**: The type of the field in the original data.
+- **`isArray`**: Whether the field is an array or not.
+
+Multiple `conversions` can be used if there are different field names or types that need to be mapped to a single VSpec field. When generating code, the system will search the document for each `originalName` until the first match is found. This allows flexibility in handling variations in field names or types from different data sources.
+
+## requiredPrivileges
+
+The `requiredPrivileges` field lists the privileges required to access the signal. This ensures that only users with the appropriate permissions can access sensitive or specific vehicle data.
+
+## Example
+
+Here's a breakdown of a sample entry in the `definitions.yaml` file:
+
+```yaml
+- vspecName: Vehicle.Chassis.Axle.Row1.Wheel.Left.Tire.Pressure
+  conversions:
+    - originalName: tires.frontLeft
+      originalType: float64
+      isArray: false
+  requiredPrivileges:
+    - VEHICLE_NON_LOCATION_DATA
+```
+
+- **`vspecName`**: This maps to the VSpec field for the left front tire pressure.
+- **`conversions`**: The `originalName` is `tires.frontLeft` and the type is `float64`. This field is not an array.
+- **`requiredPrivileges`**: Access requires the `VEHICLE_NON_LOCATION_DATA` privilege.

pkg/schema/validation.go

@@ -8,42 +8,42 @@ import (
 // privileges are defined on chain and copied here for validation.
 var privileges = []string{"VEHICLE_NON_LOCATION_DATA", "VEHICLE_COMMANDS", "VEHICLE_CURRENT_LOCATION", "VEHICLE_ALL_TIME_LOCATION", "VEHICLE_VIN_CREDENTIAL"}
 
-// ErrInvalid is an error for invalid definitions.
-type ErrInvalid struct {
+// InvalidError is an error for invalid definitions.
+type InvalidError struct {
 	Property string
 	Name     string
 	Reason   string
 }
 
-func (e ErrInvalid) Error() string {
-	return fmt.Sprintf("signal '%s' property '%s' %s", e.Property, e.Name, e.Reason)
+func (e InvalidError) Error() string {
+	return fmt.Sprintf("signal '%s' property '%s' %s", e.Name, e.Property, e.Reason)
 }
 
 // Validate checks if the definition is valid.
 func Validate(d *DefinitionInfo) error {
 	if d == nil {
-		return ErrInvalid{Property: "", Name: "", Reason: "is nil"}
+		return InvalidError{Property: "", Name: "", Reason: "is nil"}
 	}
 	if d.VspecName == "" {
-		return ErrInvalid{Property: "vspecName", Name: d.VspecName, Reason: "is empty"}
+		return InvalidError{Property: "vspecName", Name: d.VspecName, Reason: "is empty"}
 	}
 	if len(d.Conversions) == 0 {
-		return ErrInvalid{Property: "conversions", Name: d.VspecName, Reason: "at least one conversion is required"}
+		return InvalidError{Property: "conversions", Name: d.VspecName, Reason: "at least one conversion is required"}
 	}
 	for _, conv := range d.Conversions {
 		if conv == nil {
-			return ErrInvalid{Property: "conversion", Name: d.VspecName, Reason: "is nil"}
+			return InvalidError{Property: "conversion", Name: d.VspecName, Reason: "is nil"}
 		}
 		if conv.OriginalName == "" {
-			return ErrInvalid{Property: "originalName", Name: d.VspecName, Reason: "is empty"}
+			return InvalidError{Property: "originalName", Name: d.VspecName, Reason: "is empty"}
 		}
 	}
 	if len(d.RequiredPrivileges) == 0 {
-		return ErrInvalid{Property: "requiredPrivileges", Name: d.VspecName, Reason: "at least one privilege is required"}
+		return InvalidError{Property: "requiredPrivileges", Name: d.VspecName, Reason: "at least one privilege is required"}
 	}
 	for _, priv := range d.RequiredPrivileges {
 		if !slices.Contains(privileges, priv) {
-			return ErrInvalid{Property: "requiredPrivileges", Name: d.VspecName, Reason: fmt.Sprintf("must be one of %v", privileges)}
+			return InvalidError{Property: "requiredPrivileges", Name: d.VspecName, Reason: fmt.Sprintf("must be one of %v", privileges)}
 		}
 	}
 	return nil

pkg/schema/validation_test.go

@@ -20,7 +20,7 @@ func TestValidate(t *testing.T) {
 		{
 			name: "Nil Definition",
 			d:    nil,
-			expected: ErrInvalid{
+			expected: InvalidError{
 				Property: "",
 				Name:     "",
 				Reason:   "is nil",
@@ -31,7 +31,7 @@ func TestValidate(t *testing.T) {
 			d: &DefinitionInfo{
 				VspecName: "",
 			},
-			expected: ErrInvalid{
+			expected: InvalidError{
 				Property: "vspecName",
 				Name:     "",
 				Reason:   "is empty",
@@ -42,7 +42,7 @@ func TestValidate(t *testing.T) {
 			d: &DefinitionInfo{
 				VspecName: "Vehicle",
 			},
-			expected: ErrInvalid{
+			expected: InvalidError{
 				Property: "conversions",
 				Name:     "Vehicle",
 				Reason:   "at least one conversion is required",
@@ -56,7 +56,7 @@ func TestValidate(t *testing.T) {
 					nil,
 				},
 			},
-			expected: ErrInvalid{
+			expected: InvalidError{
 				Property: "conversion",
 				Name:     "Vehicle",
 				Reason:   "is nil",
@@ -70,7 +70,7 @@ func TestValidate(t *testing.T) {
 					{OriginalName: ""},
 				},
 			},
-			expected: ErrInvalid{
+			expected: InvalidError{
 				Property: "originalName",
 				Name:     "Vehicle",
 				Reason:   "is empty",
@@ -82,7 +82,7 @@ func TestValidate(t *testing.T) {
 				VspecName:   "Vehicle",
 				Conversions: []*ConversionInfo{{OriginalName: "OriginalName"}},
 			},
-			expected: ErrInvalid{
+			expected: InvalidError{
 				Property: "requiredPrivileges",
 				Name:     "Vehicle",
 				Reason:   "at least one privilege is required",
@@ -95,7 +95,7 @@ func TestValidate(t *testing.T) {
 				Conversions:        []*ConversionInfo{{OriginalName: "OriginalName"}},
 				RequiredPrivileges: []string{"INVALID_PRIVILEGE"},
 			},
-			expected: ErrInvalid{
+			expected: InvalidError{
 				Property: "requiredPrivileges",
 				Name:     "Vehicle",
 				Reason:   "must be one of [VEHICLE_NON_LOCATION_DATA VEHICLE_COMMANDS VEHICLE_CURRENT_LOCATION VEHICLE_ALL_TIME_LOCATION VEHICLE_VIN_CREDENTIAL]",