Skip to content
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

Unable to save on UI Advanced Settings form #2256

Open
justjam2013 opened this issue Nov 27, 2024 · 7 comments
Open

Unable to save on UI Advanced Settings form #2256

justjam2013 opened this issue Nov 27, 2024 · 7 comments
Labels

Comments

@justjam2013
Copy link
Contributor

justjam2013 commented Nov 27, 2024

Describe The Bug

I opened the UI Advanced Settings form. The red exclamation sign was displayed and the Save button was disabled.
I checked all the sections on the form and the only field marked as required is the Homebridge UI Authentication dropdown, which has Require Authentication selected:

Screenshot 2024-11-27 at 10 41 27 PM

I tried selecting None from the dropdown, then Require Authentication again, but there was no change in the state of the Save button.

I tried entering values in every single field and selecting every single checkbox on the form, but there was no change in the state of the Save button.

Logs

No response

Config

No response

Homebridge UI Version

5.0.0-beta.13

Homebridge Version

1.8.5

Node.js Version

22.9.0

Operating System

macOS

Environment Info

Using hb-service

Raspberry Pi Model

None

@justjam2013
Copy link
Contributor Author

Compared my prod instance with my dev instance

  • Prod: Save enabled on UI Advanced Settings modal dialog
  • Dev: Save disabled on UI Advanced Settings modal dialog

Went digging through the code and eventually got to homebridge-config-ui-x/config.schema.json.
Of course, homebridge-config-ui-x is a plugin. Hidden, but a plugin nonetheless!

Compared the JSON config for the two instances.

No differences in the bridge section:

Prod:

    "bridge": {
        "name": "Homebridge 4FAF",
        "username": "0E:25:8B:8B:4F:AF",
        "port": 51732,
        "pin": "XXX-XX-XXX",
        "advertiser": "bonjour-hap"
    },

Dev:

    "bridge": {
        "name": "Homebridge D459",
        "username": "0E:E3:8A:15:D4:59",
        "port": 51046,
        "pin": "XXX-XX-XXX",
        "advertiser": "bonjour-hap"
    },

Compared the platforms section:

Prod:

    "platforms": [
        {
            "name": "Config",
            "port": 8581,
            "auth": "form",
            "theme": "dark-mode",
            "tempUnits": "f",
            "lang": "auto",
            "platform": "config"
        },
        ...
    ]

Dev:

    "platforms": [
        {
            "name": "Config",
            "port": 8581,
            "platform": "config",
            "tempUnits": "f",
            "theme": "orange",
            "lightingMode": "auto"
        },
        ...
    ]

My Dev instance is missing values for lang and auth.

However, Language is set to Automatic:
Screenshot 2024-12-12 at 12 50 47 PM

And Authentication is set to Require Authentication:
Screenshot 2024-12-12 at 12 51 06 PM

My guess is that because Automatic is the default value for Language and Require Authentication is the default value for Authentication, I did not specifically select the values in the dropdowns. So when the form was initially submitted (pre form validation changes), those values were not saved to the JSON config.

When Homebridge UI is started, it assumes those two fields to be the default values, but when it verifies the UI Advanced Settings modal dialog, iti does not. It simply sees those two values as missing and blocks saving.

@justjam2013
Copy link
Contributor Author

justjam2013 commented Dec 12, 2024

Options:

Option 1:

Remove strict validation

"strictValidation": true,

Note: This is not an option because that defeats the purpose of validating that the saved values are logically meaningful for the plugin.

Option 2:

Remove required for auth field:

      "auth": {
        ...
        "required": true
      },

and from lang field:

      "lang": {
        ...
        "required": true,
        ...
      },

"required": true,

Note: This is also not an option because that defeats the purpose of validating that the saved values are logically meaningful for the plugin and also because the following fields are required and have default values:

  • theme
  • tempUnits

Option 3:

When saving the form, check that all required values are present.

Note: This is going to unnecessarily complicate the code for maintenance

Option 4:

Provide the default values in the definition of originalUiSettingsForm:

public originalUiSettingsForm = {
port: 0,
}

and

  • if the new value (data[key]) is undefined or "", then do not overwrite the original value (this.originalUiSettingsForm[key]):
  • save all of the fields
    Object.keys(data).forEach((key) => {
    if (this.originalUiSettingsForm[key] !== data[key]) {
    this.saveUiSettingChange(key, data[key])
    hasChangedUiSettings = true
    }
    })

@justjam2013
Copy link
Contributor Author

justjam2013 commented Dec 12, 2024

This may need to be updated with the default values for the required fields:

/**
* Create the default ui config
*/
private async createDefaultUiConfig() {
return {
name: 'Config',
port: this.action === 'install' ? this.uiPort : await this.getLastKnownUiPort(),
platform: 'config',
}
}

After reviewing the code, I don't believe this should need to be changed as this is called when a config doesn't exist yet.

@bwp91
Copy link
Contributor

bwp91 commented Dec 27, 2024

Hi @justjam2013

I am unable to recreate this from your ideas.

I've trimmed my config down to the following for config-ui-x (and restarted)

        {
            "name": "Config",
            "port": 8581,
            "platform": "config"
        },

Yet when I go to the advanced settings form I still see a green tick at the bottom.

There are some advanced settings that are only available when not running in service mode, which are filtered out from the config.schema.json file here:

// modify this plugins schema to set the default port number
if (pluginName === this.configService.name) {
configSchema.schema.properties.port.default = this.configService.ui.port
// filter some options from the UI config when using service mode
if (this.configService.serviceMode) {
configSchema.layout = configSchema.layout.filter((x: any) => {
return x.ref !== 'log'
})
configSchema.layout = configSchema.layout.filter((x: any) => {
return !(x === 'sudo' || x.key === 'restart')
})
}
}

So I wonder if this has anything to do with the exclamation mark that you were seeing? Just a thought.

@justjam2013
Copy link
Contributor Author

justjam2013 commented Dec 27, 2024

I have updated my "Dev" instance to HB UI v5.0.0-beta.24 and Node.js 22.12.0 since opening this ticket (see below).
Even adding auth and lang to the platform section, the red exclamation sign is still displayed and the Save button is still disabled.

Dev setup:
Homebridge UI Version: 5.0.0-beta.24
Homebridge Version: 1.8.5
Node.js Version: 22.12.0
Operating System: macOS
Environment Info: Using hb-service

I have tried replacing the platform section with the one from the "Prod" instance, no change.

Prod setup:
Homebridge UI Version: 4.65.2
Homebridge Version: 1.8.5
Node.js Version: 22.12.0
Operating System: Ubuntu Noble (24.04.1 LTS)
Environment Info: Docker official Homebridge image

Any thoughts on what else I can check?
I have added the two configurations below and as you can see, there are very minor differences between them. Yet the Prod instance allows saving UI Advanced Settings, the Dev instance doesn't.

===============================================================

Dev Config:

{
    "bridge": {
        "name": "Homebridge D459",
        "username": "0E:E3:8A:15:D4:59",
        "port": 51046,
        "pin": "XXX-XX-XXX",
        "advertiser": "bonjour-hap"
    },
    "accessories": [],
    "platforms": [
        {
            "name": "Config",
            "port": 8581,
            "platform": "config",
            "tempUnits": "f",
            "theme": "orange",
            "lightingMode": "auto"
        },
        {
            ...
            "platform": "VirtualAccessoriesForHomebridge"
        }
    ]
}

Prod Config:

{
    "bridge": {
        "name": "Homebridge 4FAF",
        "username": "0E:25:8B:8B:4F:AF",
        "port": 51732,
        "pin": "XXX-XX-XXX",
        "advertiser": "bonjour-hap"
    },
    "accessories": [
        ...
    ],
    "platforms": [
        {
            "name": "Config",
            "port": 8581,
            "auth": "form",
            "theme": "dark-mode",
            "tempUnits": "f",
            "lang": "auto",
            "platform": "config"
        },
        {
            ...
            "platform": "RandomDelaySwitches"
        },
        {
            ...
            "platform": "esphome"
        },
        {
            ...
            "platform": "HomepodRadioPlatform"
        },
        {
            ...
            "platform": "VirtualAccessoriesForHomebridge"
        }
    ],
    "disabledPlugins": []
}

@bwp91
Copy link
Contributor

bwp91 commented Dec 27, 2024

In your 'prod' config, "theme": "dark-mode", this theme does not exist. I tested this, and was leading to the red !.

But this does not align with

Yet the Prod instance allows saving UI Advanced Settings, the Dev instance doesn't.

Since I would expect your prod instance to show the red ! due to the incorrect theme.

@justjam2013
Copy link
Contributor Author

justjam2013 commented Dec 27, 2024

In your 'prod' config, "theme": "dark-mode", this theme does not exist. I tested this, and was leading to the red !.

The value "dark-mode" is from an older version of Homebridge UI, as I distinctly remember when the UI changed to Orange, but as I liked the orange I actually never manually changed it myself. And as I never changed it to any other color, it was never updated in the config.
When I checked the Theme dropdown, it is set to Orange, which is the default option. My guess is that when the save occurs, if no value is explicitly selected, the form returns a value of either "" (empty string) or undefined for "theme". And iirc the code checks to see if the value returned from the form is different from the saved value, but it excludes fields with no value.

Screenshot 2024-12-27 at 6 36 59 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants