Conversation
There was a problem hiding this comment.
Hey @andig - I've reviewed your changes - here's some feedback:
- Move the demonstration files (
test.goandhandler_loadpoint_status.go) into anexamples/folder or remove them—production code shouldn’t include standalone example mains. - Consolidate URI extraction helpers (
extractID,extractNumericID,extractUserIDFromURI, etc.) into a single utility to avoid duplicated logic across the codebase. - Uncomment and configure the CORS and compression middleware for the
/mcproute so clients can access the MCP API reliably and with optimal performance.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Move the demonstration files (`test.go` and `handler_loadpoint_status.go`) into an `examples/` folder or remove them—production code shouldn’t include standalone example mains.
- Consolidate URI extraction helpers (`extractID`, `extractNumericID`, `extractUserIDFromURI`, etc.) into a single utility to avoid duplicated logic across the codebase.
- Uncomment and configure the CORS and compression middleware for the `/mcp` route so clients can access the MCP API reliably and with optimal performance.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@andig I'm working on further implementing based on what you started. Once i've got it together I'll push, can I push to this branch? Not sure.. On the questions: Tools vs Resources: A Practical PhilosophyThe implementation demonstrates a clear conceptual model that goes beyond mere categorization. What strikes me is how the distinction naturally emerges from the domain itself: Resources create a navigable information architecture: // Hierarchical, discoverable state representation
"loadpoints://{loadpoint_id}" // Individual charging point states
"vehicles://list" // Vehicle fleet overview
"metrics://current" // Real-time energy flows
"battery://status" // Storage system telemetryTools orchestrate state transitions with clear intent: // Explicit, purposeful actions
"start-charging" // Initiates energy transfer
"set-charge-mode" // Defines optimization strategy
"set-charge-plan" // Temporal coordination (80% by 7am)
"set-priority" // Resource allocation logicThis separation is not only about safety or API design, it also creates a language for energy management that both humans and AI can reason about effectively. External Resources: Trusting the WebThe external documentation resource reveals an elegant truth about distributed systems: s.AddResource(
mcp.NewResource(
"https://docs.evcc.io",
"docs",
mcp.WithResourceDescription("evcc documentation"),
),
nil, // TODO no handler needed
)That Each external resource enriches the AI's context without coupling evcc to external services other than those already implemented to have evcc function as it does. |
|
Architecture: I'm not convinced that it is a good idea to connect the MCP adapter directly to our core and open another full-control channel to the outside. I'd rather see it as an adapter/bridge that provides more fine grain and use-case specific interaction built ontop of our existing external interface (e.g. REST API). This way we can e.g. reuse authentication and make testing this a lot easier. |
@nickels you'll need to target this branch in a new PR. Really looking forward to it! For each addition: can we add prompts that should be supported now? I've tried adding the MCP to Claude Desktop (and change my plan) but I can't get Claude to actually produce answers.
Total number of loadpoints would really be a resource then? Or rather- wo we even need that or should we simply return all loadpoints? Is the representation of loadpoints as (slice of) JSON even helpful?
I would agree if it didn't create a panic, see mark3labs/mcp-go#422. Maybe an implementation restriction. I could add an "empty response" handler but would that allow the client to actually fetch the docs? |
@naltatis @nickels I'd treat this as first experiment, not design. I've seem other posts turning a OpenAPI spec into a consumable MCP server. If agents had the ability to consume OpenAPI we might skip any of that- but would need to provide an OpenAPI spec. Might be helpful anyway but establishes a new contract. For time being- should we continue as-is (or prototype an additional approach)? |
We already have that: https://docs.evcc.io/docs/integrations/rest-api Only thing missing there is an extensive documentation for the But we already have plans to auto-generate a JSON schema with explanations from the newly introduced TypeScript structure (see here). It still might be a good idea to add another, more use-case focused abstraction ontop of the /api/state endpoint that's easier to consume: "Whats the charging state of vehicle B?" \cc @Maschga |
PR answers this for a loadpoint ;). However imho its up to the AI to find out via vehicle-loadpoint relation. |
|
Creating an OpenAPI wrapper depends on getkin/kin-openapi#1079 |
|
No new insights, closing. |
Refs #21921
Questions: