-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
UI: EEBUS: configure by default #26882
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UI: EEBUS: configure by default #26882
Conversation
|
We should also publish the ski of the server and show it on the HEMS page (separate PR?). @Maschga can I close my PR? |
|
My plan was to merge this PR with the UI in yours and then merge yours into master. |
I think @DerAndereAndi mentioned the QR code which contains the SKI. The controlbox frontend shows it. Here is an example:
Additional information on QR code:
|
|
As you like. You can also take over mine. I can always add here 😅 |
|
Then we can gladly continue here together. 👍 |
There was a problem hiding this 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.Maskedhelper takesanybut compares directly to"", which will panic for non-string types; either narrow its parameter type tostringor perform a safe type assertion/conversion before comparison. - In
EebusModal.vue, thePropertyCertFieldcomponents are passed:model-valuewithout a correspondingv-model/update handler, so certificate changes likely won't propagate back intovalues.certificate; consider wiring them with two-way binding consistent with other form fields. - The
certificatefield in the TSEebustype is optional while the GoConfigstruct always has aCertificatevalue; ifcertificateis guaranteed server-side, consider making it non-optional in TS or aligning the JSON tags withomitemptyto 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>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 { | |||
There was a problem hiding this comment.
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.

Ref #26615
TODO:
\cc @andig @naltatis