Fix: Follow Go naming conventions

codegen/README.md

@@ -31,5 +31,14 @@ in CI keeps it in sync with the schema. See `xbot_service_interface_go/codegen`
 implementation and the golden tests.
 
 Not supported yet: outputs (DATA received from the service) need a receive loop in the Go client,
-and arrays other than `char[n]`/`uint8_t[n]` are rejected while generating (loudly) instead of
-producing broken code.
+and unknown array element types are rejected while generating (loudly) instead of producing broken
+code.
+
+Identifiers are translated to the Go naming rules for initialisms (`SoundId` -> `SoundID`,
+`GpsQuality` -> `GPSQuality`; see `xbot_service_interface_go/codegen/names.go`). The original name
+from the `service.json` stays visible, so the schema remains the reference a reader greps for:
+
+- doc comments on types, RPC methods, register setters and input senders
+  (`// PlayMP3 calls the RPC function 3 ("PlayMp3" in the service.json).`),
+- trailing comments on the enum values (`SoundIDBootPing SoundID = 0 // BOOT_PING`),
+- nothing on the `Fn*` function-id constants: they are only used together with the RPC method whose doc comment names the schema function.

xbot_service_interface_go/README.md

@@ -62,14 +62,14 @@ Pass a context with a deadline, or set `Options.CallTimeout` — otherwise `Defa
 
 ### `xbot.Dial(ctx, xbot.Options{...}) (*xbot.Conn, error)`
 
-| field         | meaning                                                                         |
-| ------------- | ------------------------------------------------------------------------------- |
-| `ServiceID`   | the id to connect to (required), see `service_ids.h`                            |
-| `BindIP`      | IPv4 address of the interface to use; `""`/`0.0.0.0` = all                      |
-| `Heartbeat`   | heartbeat interval; `0` = `xbot.DefaultHeartbeat` (1 s)                         |
-| `NoClaim`     | connect without claiming: the owner keeps the responses, this client only sends |
-| `Endpoint`    | `"ip:port"` — skip discovery (implies `NoClaim`)                                |
-| `CallTimeout` | default timeout for `Call` without a context deadline; `0` = 2 s                |
+| field                         | meaning                                                                          |
+| ----------------------------- | -------------------------------------------------------------------------------- |
+| `ServiceID`                   | the id to connect to (required), see `service_ids.h`                             |
+| `BindIP`                      | IPv4 address of the interface to use; `""`/`0.0.0.0` = all                       |
+| `Heartbeat`                   | heartbeat interval; `0` = `xbot.DefaultHeartbeat` (1 s)                          |
+| `NoClaim`                     | connect without claiming: the owner keeps the responses, this client only sends  |
+| `Endpoint`                    | `"ip:port"` — skip discovery (implies `NoClaim`)                                 |
+| `CallTimeout`                 | default timeout for `Call` without a context deadline; `0` = 2 s                 |
 | `ExpectType`, `ExpectVersion` | validate the discovered service against the schema the client was generated from |
 
 Discovery needs a window of a few advertisement intervals: a service advertises every second only while it is
@@ -97,12 +97,12 @@ descriptor.
 
 ### Errors
 
-| error                   | meaning                                                            |
-| ----------------------- | ------------------------------------------------------------------ |
-| `xbot.ErrBusy`          | the service is still executing another call (`errors.Is`)          |
-| `xbot.ErrRPC`           | the call failed inside the service                                 |
-| `xbot.ErrNotAdvertised` | the service id was not advertised; the message lists what was seen |
-| `*xbot.RPCError`        | carries `FunctionID` and `Status`; matches the two above via `Is`  |
+| error                         | meaning                                                                                        |
+| ----------------------------- | ---------------------------------------------------------------------------------------------- |
+| `xbot.ErrBusy`                | the service is still executing another call (`errors.Is`)                                      |
+| `xbot.ErrRPC`                 | the call failed inside the service                                                             |
+| `xbot.ErrNotAdvertised`       | the service id was not advertised; the message lists what was seen                             |
+| `*xbot.RPCError`              | carries `FunctionID` and `Status`; matches the two above via `Is`                              |
 | `xbot.ErrIncompatibleService` | the service announced a different type/version than the client expects (`*xbot.MismatchError`) |
 
 ### Discovery and helpers
@@ -135,6 +135,29 @@ The output is `gofmt`ed and checked in (Go convention); a `go:generate` directiv
 `go generate ./... && git diff --exit-code` in CI keeps it in sync with the schema. Generator and
 golden tests live in `codegen/`.
 
+**Naming.** The schema is the reference, the Go API follows the Go naming rules for initialisms:
+`SoundId` -> `SoundID`, `GpsQuality` -> `GPSQuality`, `PlayMp3` -> `PlayMP3`, `I2cReceive` ->
+`I2CReceive`, `GPS_RTK_FIX` -> `GPSRTKFix`. That translation is mechanical (see `codegen/names.go`,
+pinned by `TestGoName`) and - because C++ style (`Gps`) and Go style (`GPS`) disagree by design -
+the original name stays visible where it matters:
+
+```go
+// PlayMP3 calls the RPC function 3 ("PlayMp3" in the service.json).
+func (c *Client) PlayMP3(...)
+
+// SendGPSQuality sends the input 3 ("Gps Quality" in the service.json).
+func (c *Client) SendGPSQuality(GPSQuality float32) error
+
+// SoundID is the service.json enum "SoundId".
+type SoundID uint8
+
+SoundIDBootPing SoundID = 0 // BOOT_PING
+```
+
+Doc comments carry it on types, RPC methods, register setters and input senders; enum values carry it as a trailing comment; the `Fn*` function-id constants have none, because they are only used with the
+RPC method whose doc comment names the schema function. So `grep` on the schema name always finds the
+Go identifier, and a reader can go from `service.json` to the client without knowing the rules.
+
 ## Scope
 
 The client implements what a tool like `soundctl` needs: discovery, claim, heartbeat, RPC (synchronous and

xbot_service_interface_go/codegen/emit.go

@@ -87,7 +87,7 @@ func (g *generator) goScalar(t, where string) (string, error) {
 		return gt, nil
 	}
 	if g.enumBase(t) != "" {
-		return t, nil
+		return goName(t), nil
 	}
 	return "", fmt.Errorf("go codegen: unsupported type %q in %s", t, where)
 }

xbot_service_interface_go/codegen/generate_test.go

@@ -55,13 +55,25 @@ func TestGenerateGolden(t *testing.T) {
 // as well, instead of producing a client that cannot work.
 func TestLoadRejects(t *testing.T) {
 	cases := map[string]string{
-		"missing type":      `{"version": 1}`,
-		"unknown type":      `{"type": "X", "version": 1, "inputs": [{"id": 0, "name": "A", "type": "uint7_t"}]}`,
-		"blob as input":     `{"type": "X", "version": 1, "inputs": [{"id": 0, "name": "A", "type": "blob"}]}`,
-		"duplicate ids":     `{"type": "X", "version": 1, "inputs": [{"id": 1, "name": "A", "type": "uint8_t"}, {"id": 1, "name": "B", "type": "uint8_t"}]}`,
-		"name collision":    `{"type": "X", "version": 1, "inputs": [{"id": 0, "name": "My Input", "type": "uint8_t"}, {"id": 1, "name": "MyInput", "type": "uint8_t"}]}`,
-		"unknown enum base": `{"type": "X", "version": 1, "enums": [{"id": "E", "base_type": "uint7_t", "values": {"A": 0}}]}`,
-		"empty enum":        `{"type": "X", "version": 1, "enums": [{"id": "E", "base_type": "uint8_t", "to_string": true, "values": {}}]}`,
+		"missing type":                   `{"version": 1}`,
+		"unknown type":                   `{"type": "X", "version": 1, "inputs": [{"id": 0, "name": "A", "type": "uint7_t"}]}`,
+		"blob as input":                  `{"type": "X", "version": 1, "inputs": [{"id": 0, "name": "A", "type": "blob"}]}`,
+		"duplicate ids":                  `{"type": "X", "version": 1, "inputs": [{"id": 1, "name": "A", "type": "uint8_t"}, {"id": 1, "name": "B", "type": "uint8_t"}]}`,
+		"name collision":                 `{"type": "X", "version": 1, "inputs": [{"id": 0, "name": "My Input", "type": "uint8_t"}, {"id": 1, "name": "MyInput", "type": "uint8_t"}]}`,
+		"unknown enum base":              `{"type": "X", "version": 1, "enums": [{"id": "E", "base_type": "uint7_t", "values": {"A": 0}}]}`,
+		"empty enum":                     `{"type": "X", "version": 1, "enums": [{"id": "E", "base_type": "uint8_t", "to_string": true, "values": {}}]}`,
+		"function shadows Client method": `{"type": "X", "version": 1, "functions": [{"id": 0, "name": "Close", "return_type": "void"}]}`,
+		"function and register share a setter": `{"type": "X", "version": 1,
+			"functions": [{"id": 0, "name": "SetVolume", "return_type": "void"}],
+			"registers": [{"id": 0, "name": "Volume", "type": "uint8_t"}]}`,
+		"functions normalize equal": `{"type": "X", "version": 1, "functions": [
+			{"id": 0, "name": "SoundId", "return_type": "void"},
+			{"id": 1, "name": "SoundID", "return_type": "void"}]}`,
+		"enum values normalize equal": `{"type": "X", "version": 1, "enums": [{"id": "E", "base_type": "uint8_t",
+			"values": {"IDLE": 0, "Idle": 1}}]}`,
+		"enum types normalize equal": `{"type": "X", "version": 1, "enums": [
+			{"id": "SoundId", "base_type": "uint8_t", "values": {"A": 0}},
+			{"id": "SoundID", "base_type": "uint8_t", "values": {"A": 0}}]}`,
 	}
 	for name, schema := range cases {
 		t.Run(name, func(t *testing.T) {

xbot_service_interface_go/codegen/names.go

@@ -0,0 +1,138 @@
+package codegen
+
+import (
+	"strings"
+	"unicode"
+	"unicode/utf8"
+)
+
+// initialismNames lists the words Go writes in one case (ID, not Id; URL, not
+// Url; Hz, not HZ), in their canonical spelling. The lookup map below is derived
+// from it, keyed by the lower case spelling, so one entry covers CamelCase
+// ("SoundId"), SCREAMING_SNAKE ("GPS_RTK_FIX") and prose ("State ID").
+var initialismNames = []string{
+	"ADC", "API", "BMS", "CAN", "CDC", "ESC", "GPS", "GPIO", "HTTP", "Hz",
+	"I2C", "I2S", "ID", "IMU", "IP", "JSON", "LED", "MP3", "PCB", "PID",
+	"PWM", "ROS", "RPC", "RTK", "SPI", "TCP", "UDP", "URL", "USB",
+}
+
+// initialisms maps the lower case spelling to the canonical one.
+var initialisms = func() map[string]string {
+	m := make(map[string]string, len(initialismNames))
+	for _, name := range initialismNames {
+		m[strings.ToLower(name)] = name
+	}
+	return m
+}()
+
+// goName converts a schema identifier into a Go identifier conforming to the Go
+// naming rules for initialisms: "SoundId" -> "SoundID", "PlayMp3" -> "PlayMP3",
+// "I2cReceive" -> "I2CReceive", "GPS_RTK_FIX" -> "GPSRTKFix", "IDLE" -> "Idle".
+//
+// Names that need no change come out unchanged, so the generator output stays
+// close to the service.json (which keeps it greppable against the C++ side).
+func goName(name string) string {
+	words := splitWords(name)
+	if len(words) == 0 {
+		return ""
+	}
+	var b strings.Builder
+	for _, w := range words {
+		b.WriteString(goWord(w))
+	}
+	return b.String()
+}
+
+// splitWords splits on whitespace (spaces, tabs, newlines), underscores and
+// dashes, and at CamelCase boundaries: "GPIOEvent" -> GPIO, Event; "I2cReceive" -> I2c, Receive;
+// "Battery Voltage ADC" -> Battery, Voltage, ADC; "GPS_RTK_FIX" -> GPS, RTK, FIX.
+func splitWords(name string) []string {
+	isSep := func(r rune) bool { return r == '_' || r == '-' || unicode.IsSpace(r) }
+	isUpper := func(r rune) bool { return r >= 'A' && r <= 'Z' }
+	isLower := func(r rune) bool { return r >= 'a' && r <= 'z' }
+	isDigit := func(r rune) bool { return r >= '0' && r <= '9' }
+
+	runes := []rune(name)
+	var words []string
+	start := -1
+	emit := func(end int) {
+		if start >= 0 && end > start {
+			words = append(words, string(runes[start:end]))
+		}
+		start = -1
+	}
+
+	for i := 0; i < len(runes); i++ {
+		r := runes[i]
+		if isSep(r) {
+			emit(i)
+			continue
+		}
+		if start < 0 {
+			start = i
+			continue
+		}
+		prev := runes[i-1]
+		switch {
+		case (isLower(prev) || isDigit(prev)) && isUpper(r):
+			// "SoundId": a new word starts at the upper case letter.
+			emit(i)
+			start = i
+		case isUpper(prev) && isUpper(r) && i+1 < len(runes) && isLower(runes[i+1]):
+			// "GPIOEvent": the last upper case letter starts the next word.
+			emit(i)
+			start = i
+		}
+	}
+	emit(len(runes))
+	return words
+}
+
+// goWord renders one word: known initialisms as they are, an all upper case run
+// of initialisms split into them (GPIOID -> GPIOID), everything else Title case
+// (IDLE -> Idle, FIX -> Fix).
+func goWord(word string) string {
+	if mapped, ok := initialisms[strings.ToLower(word)]; ok {
+		return mapped
+	}
+	if word == strings.ToUpper(word) {
+		if parts, ok := splitInitialisms(word); ok {
+			return parts
+		}
+	}
+	// Title case, decoding the first rune so accented letters survive.
+	r, size := utf8.DecodeRuneInString(word)
+	if r == utf8.RuneError && size <= 1 {
+		return word
+	}
+	return string(unicode.ToUpper(r)) + strings.ToLower(word[size:])
+}
+
+// splitInitialisms splits an all upper case word greedily into initialisms
+// ("GPIOID" -> "GPIO"+"ID"). It reports false unless the whole word is covered,
+// so a normal word in capitals (IDLE) is not mangled into "ID"+"LE".
+func splitInitialisms(word string) (string, bool) {
+	lower := strings.ToLower(word)
+	var b strings.Builder
+	parts := 0
+	for pos := 0; pos < len(lower); {
+		best := ""
+		for candidate := range initialisms {
+			if len(candidate) > len(best) && strings.HasPrefix(lower[pos:], candidate) {
+				best = candidate
+			}
+		}
+		if best == "" {
+			return "", false
+		}
+		b.WriteString(initialisms[best])
+		pos += len(best)
+		parts++
+	}
+	// A word that is a single initialism is already handled by the table lookup
+	// in goWord, so only report a real split.
+	if parts < 2 {
+		return "", false
+	}
+	return b.String(), true
+}

xbot_service_interface_go/codegen/names_test.go

@@ -0,0 +1,58 @@
+package codegen
+
+import "testing"
+
+// TestGoName pins the naming rules against the identifiers that actually occur in
+// the OpenMower service definitions plus the tricky shapes (all caps runs,
+// digits, prose with spaces).
+func TestGoName(t *testing.T) {
+	cases := map[string]string{
+		// initialisms in CamelCase
+		"SoundId":           "SoundID",
+		"PlayMp3":           "PlayMP3",
+		"BusID":             "BusID",
+		"GPIOID":            "GPIOID",
+		"GpsQuality":        "GPSQuality",
+		"I2cReceive":        "I2CReceive",
+		"I2cTransmit":       "I2CTransmit",
+		"I2cResult":         "I2CResult",
+		"BatteryVoltageADC": "BatteryVoltageADC",
+		"ReadGPIO":          "ReadGPIO",
+		"SubscribeGPIO":     "SubscribeGPIO",
+		"GPIOEventFlags":    "GPIOEventFlags",
+		// constant style: SCREAMING_SNAKE
+		"GPS_RTK_FIX":     "GPSRTKFix",
+		"ROS_CONNECTED":   "ROSConnected",
+		"RosDisconnected": "ROSDisconnected",
+		"EscStatus":       "ESCStatus",
+		"GPS_RTK_LOST":    "GPSRTKLost",
+		"BOOT_PING":       "BootPing",
+		"IDLE":            "Idle",
+		"MP3":             "MP3",
+		"TONE":            "Tone",
+		// prose names from the registers
+		"State ID":            "StateID",
+		"Gps Quality":         "GPSQuality",
+		"GPIO Configs":        "GPIOConfigs",
+		"Battery Voltage ADC": "BatteryVoltageADC",
+		"Charger Enabled":     "ChargerEnabled",
+		"Sound Definitions":   "SoundDefinitions",
+		"Simulated Inputs":    "SimulatedInputs",
+		// any whitespace separates words, not just the space
+		"Battery\tVoltage":   "BatteryVoltage",
+		"Sound\nDefinitions": "SoundDefinitions",
+		"Gps  Quality":       "GPSQuality",
+		// untouched names
+		"Path":       "Path",
+		"Hash":       "Hash",
+		"DurationMs": "DurationMs",
+		"DetuneHz":   "DetuneHz",
+		"FileExists": "FileExists",
+		"Waveform":   "Waveform",
+	}
+	for in, want := range cases {
+		if got := goName(in); got != want {
+			t.Errorf("goName(%q) = %q, want %q", in, got, want)
+		}
+	}
+}