Skip to content

quickfix for config "validate" interfering with rebasing logic#218

Closed
aldenh-viam wants to merge 1 commit intoviamrobotics:mainfrom
aldenh-viam:push-yqolxtrwovut
Closed

quickfix for config "validate" interfering with rebasing logic#218
aldenh-viam wants to merge 1 commit intoviamrobotics:mainfrom
aldenh-viam:push-yqolxtrwovut

Conversation

@aldenh-viam
Copy link
Contributor

Extremely extremely gross quickfix for the issue with #177 reported on Slack. Might be better as a starting point for a bigger redisign rather than merging as-is.

It was reported that when re-entering provisioning after we have already been online at some point in the past, some specified values in viam_defaults.json like HotspotPrefix aren't being used. This is because:

validateConfig that runs after we retrieve config from cloud and enforces that some values (like HotspotPrefix) must be present:

func validateConfig(cfg AgentConfig) (AgentConfig, error) {

If not, it substitutes in the hardcoded default:

DefaultConfiguration = AgentConfig{

This makes it difficult to "rebase" the config on the fly, like I attempted to do before (because we can't tell which values actually came from user and which are defaults that validateConfig subbed in).
Remember: we want DefaultConfig+CloudConfig for regular use, but exchange it for DefaultConfig+viam_defaults.json+CloudConfig just before entering provisioning.

The simplest solution I could come up with, implemented here, is to go through each value of the rebased config, and if it matches the hardcoded DefaultConfig, replace it with the value from the DefaultConfig+viam_defaults.json config (i.e. its value either comes from viam_defaults if specified there, otherwise DefaultConfig). This unfortunately also means if the user has specified that option in the cloud cfg and its value happens to match the DefaultConfig's, viam_defaults's will still be prioritized.

Another solution is to store the two configs separately.... unfortunately that probably means another config_cache.json on disk.

@cheukt
Copy link
Member

cheukt commented Mar 20, 2026

ya lets revert #177 instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants