Skip to content

Conversation

@Maschga
Copy link
Collaborator

@Maschga Maschga commented Jan 21, 2026

Ref #26615

TODO:

  • mask private key
  • read/write userflow
  • improve i18n strings
  • add tests
grafik

\cc @andig @naltatis

@evcc-bot evcc-bot added enhancement New feature or request ux User experience/ interface labels Jan 21, 2026
@andig
Copy link
Member

andig commented Jan 21, 2026

We should also publish the ski of the server and show it on the HEMS page (separate PR?).

@Maschga can I close my PR?

@Maschga
Copy link
Collaborator Author

Maschga commented Jan 21, 2026

My plan was to merge this PR with the UI in yours and then merge yours into master.
But you could also put my PR on master, that should work too.
I'll leave that up to you.

@CiNcH83
Copy link
Contributor

CiNcH83 commented Jan 21, 2026

We should also publish the ski of the server and show it on the HEMS page (separate PR?).

I think @DerAndereAndi mentioned the QR code which contains the SKI.

The controlbox frontend shows it. Here is an example:

image
SHIP;SKI:40ca8dfd55111a8628bc0ae150f880f83dda5a1f;ID:ControlBox Simulator SN-123456789;BRAND:Demo;TYPE:ElectricitySupplySystem;MODEL:ControlBox;SERIAL:123456789;CAT:1;ENDSHIP;

Additional information on QR code:

  1. Get QR code string from eebus-go for service [LINK]
  2. Pass string into QrcodeVue [qrcode.vue] component

@andig
Copy link
Member

andig commented Jan 21, 2026

As you like. You can also take over mine. I can always add here 😅

@Maschga
Copy link
Collaborator Author

Maschga commented Jan 21, 2026

Then we can gladly continue here together. 👍

@Maschga Maschga marked this pull request as ready for review January 21, 2026 21:34
@Maschga Maschga marked this pull request as draft January 21, 2026 21:35
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • The new util.Masked helper takes any but compares directly to "", which will panic for non-string types; either narrow its parameter type to string or perform a safe type assertion/conversion before comparison.
  • In EebusModal.vue, the PropertyCertField components are passed :model-value without a corresponding v-model/update handler, so certificate changes likely won't propagate back into values.certificate; consider wiring them with two-way binding consistent with other form fields.
  • The certificate field in the TS Eebus type is optional while the Go Config struct always has a Certificate value; if certificate is guaranteed server-side, consider making it non-optional in TS or aligning the JSON tags with omitempty to avoid inconsistent assumptions.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new `util.Masked` helper takes `any` but compares directly to `""`, which will panic for non-string types; either narrow its parameter type to `string` or perform a safe type assertion/conversion before comparison.
- In `EebusModal.vue`, the `PropertyCertField` components are passed `:model-value` without a corresponding `v-model`/update handler, so certificate changes likely won't propagate back into `values.certificate`; consider wiring them with two-way binding consistent with other form fields.
- The `certificate` field in the TS `Eebus` type is optional while the Go `Config` struct always has a `Certificate` value; if `certificate` is guaranteed server-side, consider making it non-optional in TS or aligning the JSON tags with `omitempty` to avoid inconsistent assumptions.

## Individual Comments

### Comment 1
<location> `util/redact.go:3` </location>
<code_context>
+package util
+
+func Masked(s any) string {
+	if s != "" {
+		return "***"
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Use a concrete `string` type instead of `any` for `Masked`

All current callers pass strings and the logic checks `s != ""`, so accepting `any` weakens the type contract and allows non-string values that will never match and skip masking. Restricting the parameter to `string` better reflects actual usage and lets the compiler catch accidental misuse.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@@ -0,0 +1,8 @@
package util

func Masked(s any) string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): Use a concrete string type instead of any for Masked

All current callers pass strings and the logic checks s != "", so accepting any weakens the type contract and allows non-string values that will never match and skip masking. Restricting the parameter to string better reflects actual usage and lets the compiler catch accidental misuse.

@andig andig marked this pull request as ready for review January 22, 2026 07:39
@andig andig merged commit 65f26ad into evcc-io:feat/eebus-always-on Jan 22, 2026
7 of 8 checks passed
@andig
Copy link
Member

andig commented Jan 22, 2026

@Maschga please create new PR from #26615 (merged your changes), then I can close #26615.

@naltatis naltatis mentioned this pull request Jan 22, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request ux User experience/ interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants