Add static ip support to settings #368
Conversation
WalkthroughAdds optional static IP support across firmware, backend, and web UI: new settings for static IP parameters; controller applies static or DHCP during WiFi setup; WebUI API can read/write these settings; frontend exposes a toggle and fields for static IP, netmask, gateway, and DNS. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebApp
participant WebUIPlugin
participant Settings
User->>WebApp: Toggle "Use Static IP", edit fields
WebApp->>WebUIPlugin: POST /api/settings (staticIpEnabled, staticIp, ...)
WebUIPlugin->>Settings: setStaticIpEnabled/setStaticIp/...
Settings-->>WebUIPlugin: Persisted
WebUIPlugin-->>WebApp: 200 OK
WebApp->>WebUIPlugin: GET /api/settings
WebUIPlugin->>Settings: read getters
WebUIPlugin-->>WebApp: JSON incl. static IP fields
sequenceDiagram
participant Controller
participant Settings
participant WiFi
Controller->>Settings: read staticIpEnabled + params
alt Static IP enabled with valid IP/netmask/gateway
Controller->>WiFi: config(ip, gateway, netmask, dns|gateway)
else Not enabled or no static IP
Controller->>WiFi: config(0,0,0) Note right of WiFi: DHCP
end
Controller->>WiFi: begin(ssid, pass)
WiFi-->>Controller: connection events
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/display/core/Settings.h (1)
171-175: Reasonable defaults; consider optional future constantization.Defaults look sane (netmask 255.255.255.0, empty gateway/DNS). Optionally, you could lift the netmask default into a named constant alongside other defaults in constants.h for consistency.
web/src/pages/Settings/index.jsx (1)
427-491: Add basic client-side IPv4 validation and required constraints when static IP is enabled.This reduces invalid submissions that would otherwise fall back to DHCP and avoids silent misconfigurations. Pattern + inputMode + required improve UX without changing backend.
Apply this diff to the static IP inputs:
- <input + <input id='staticIp' name='staticIp' type='text' className='input input-bordered w-full' placeholder='192.168.1.100' - value={formData.staticIp} - onChange={onChange('staticIp')} + value={formData.staticIp} + onChange={onChange('staticIp')} + autoComplete='off' + inputMode='numeric' + pattern='^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])$' + title='Enter a valid IPv4 address, e.g. 192.168.1.100' + required={!!formData.staticIpEnabled} disabled={!formData.staticIpEnabled} /> @@ - <input + <input id='staticNetmask' name='staticNetmask' type='text' className='input input-bordered w-full' placeholder='255.255.255.0' - value={formData.staticNetmask} - onChange={onChange('staticNetmask')} + value={formData.staticNetmask} + onChange={onChange('staticNetmask')} + autoComplete='off' + inputMode='numeric' + pattern='^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])$' + title='Enter a valid IPv4 netmask, e.g. 255.255.255.0' + required={!!formData.staticIpEnabled} disabled={!formData.staticIpEnabled} /> @@ - <input + <input id='staticGateway' name='staticGateway' type='text' className='input input-bordered w-full' placeholder='192.168.1.1' - value={formData.staticGateway} - onChange={onChange('staticGateway')} + value={formData.staticGateway} + onChange={onChange('staticGateway')} + autoComplete='off' + inputMode='numeric' + pattern='^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])$' + title='Enter a valid IPv4 gateway, e.g. 192.168.1.1' + required={!!formData.staticIpEnabled} disabled={!formData.staticIpEnabled} /> @@ - <input + <input id='staticDns' name='staticDns' type='text' className='input input-bordered w-full' placeholder='8.8.8.8' - value={formData.staticDns} - onChange={onChange('staticDns')} + value={formData.staticDns} + onChange={onChange('staticDns')} + autoComplete='off' + inputMode='numeric' + pattern='^(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])$' + title='Enter a valid IPv4 DNS server (optional). If omitted, the gateway will be used.' disabled={!formData.staticIpEnabled} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
src/display/core/Controller.cpp(1 hunks)src/display/core/Settings.cpp(3 hunks)src/display/core/Settings.h(3 hunks)src/display/plugins/WebUIPlugin.cpp(2 hunks)web/src/pages/Settings/index.jsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/display/core/Settings.h (1)
src/display/core/Settings.cpp (10)
setStaticIpEnabled(198-201)setStaticIpEnabled(198-198)setStaticIp(203-206)setStaticIp(203-203)setStaticNetmask(208-211)setStaticNetmask(208-208)setStaticGateway(213-216)setStaticGateway(213-213)setStaticDns(218-221)setStaticDns(218-218)
web/src/pages/Settings/index.jsx (2)
web/src/pages/Scales/index.jsx (1)
key(6-6)web/src/pages/OTA/index.jsx (1)
formData(10-10)
🔇 Additional comments (8)
src/display/core/Settings.h (2)
44-48: LGTM: getters for static IP settings are coherent and consistent with existing API.Naming, return types, and placement next to WiFi fields are consistent with the rest of Settings. No issues found.
106-110: LGTM: setters align with persistence model.The setters mirror existing style and are implemented in Settings.cpp with save() calls. No issues found.
web/src/pages/Settings/index.jsx (1)
90-92: LGTM: boolean toggle handling for staticIpEnabled matches existing pattern.Using presence toggles like other checkbox fields keeps the UI/POST shape consistent.
src/display/plugins/WebUIPlugin.cpp (2)
336-344: Boolean handling and selective updates look good; matches UI behavior.
- staticIpEnabled derived from presence of the checkbox is consistent with other toggles.
- IP fields are only updated when present, so disabled inputs won’t clobber stored values.
No changes required.
422-426: LGTM: GET now surfaces static IP settings for the UI and export.Fields align with Settings getters and match the web form names. No issues found.
src/display/core/Settings.cpp (3)
24-29: LGTM: preference keys and defaults for static IP settings are reasonable.Keys si_e, si_ip, si_nm, si_gw, si_dns are concise and consistent with existing naming.
198-221: LGTM: setters persist changes via save().Implementation is straightforward and mirrors other settings. No issues found.
436-441: LGTM: doSave persists static IP fields with existing preferences.No issues found. Keys match constructor load.
|
|
||
| // Configure static IP if enabled | ||
| if (settings.isStaticIpEnabled() && !settings.getStaticIp().isEmpty()) { | ||
| IPAddress ip, netmask, gateway, dns; | ||
|
|
||
| if (ip.fromString(settings.getStaticIp()) && | ||
| netmask.fromString(settings.getStaticNetmask()) && | ||
| gateway.fromString(settings.getStaticGateway())) { | ||
|
|
||
| if (settings.getStaticDns().isEmpty() || !dns.fromString(settings.getStaticDns())) { | ||
| dns = gateway; // Use gateway as DNS if no DNS specified | ||
| } | ||
|
|
||
| ESP_LOGI(LOG_TAG, "Configuring static IP: %s", settings.getStaticIp().c_str()); | ||
| WiFi.config(ip, gateway, netmask, dns); | ||
| } else { | ||
| ESP_LOGE(LOG_TAG, "Invalid static IP configuration, using DHCP"); | ||
| } | ||
| } else { | ||
| // Reset to DHCP | ||
| WiFi.config(0U, 0U, 0U); | ||
| } |
There was a problem hiding this comment.
Bug: invalid static IP config logs “using DHCP” but never resets to DHCP.
If parsing fails, the previous static configuration may persist. Call WiFi.config(0U,0U,0U) in the error branch to guarantee DHCP fallback.
Apply this diff:
} else {
- ESP_LOGE(LOG_TAG, "Invalid static IP configuration, using DHCP");
+ ESP_LOGE(LOG_TAG, "Invalid static IP configuration, using DHCP");
+ // Ensure we fall back to DHCP when static configuration is invalid
+ WiFi.config(0U, 0U, 0U);
}🤖 Prompt for AI Agents
In src/display/core/Controller.cpp around lines 156 to 177, the error branch
that logs "Invalid static IP configuration, using DHCP" does not actually reset
networking to DHCP, so previous static settings may persist; update the else
branch that handles failed IP parsing to call WiFi.config(0U, 0U, 0U) (after the
ESP_LOGE) to force DHCP fallback, keeping the log message as-is to indicate the
change.
2 participants
This PR adds new settings for static IP address. This is particularly useful for setups where the router does not support static IPs and mDNS does not work. This allows me to define a hardcoded hostname and ensure that Gaggimate's IP stays the same.
I tested this settings by manually flashing the firmware on the controller and the display:
Summary by CodeRabbit