Conversation
Backend ======= - Ethernet Settings: add _wifiDisabledByEthernet, disable WiFi if ethernet - Shared WebSocket: do not send message if serializeJson failed - Drivers: if pin 1 or 3 used, disable logging
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughEthernetSettingsService now disables WiFi when Ethernet becomes active (tracked by a new flag) and avoids re-enabling it on Ethernet loss. WiFiSettingsService avoids STA reconnection while Ethernet is connected. SharedWebSocketServer early-returns on empty payloads and reuses a c_str() pointer. ModuleDrivers suppresses UART0 when LED pins overlap TX/RX. ModuleIO uses board-variant pin defaults. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/framework/EthernetSettingsService.cpp`:
- Around line 130-139: The bug is that _wifiDisabledByEthernet gets set
regardless of whether WiFi was already off, causing unnecessary re-enable later;
update the Ethernet-connected branch in EthernetSettingsService (the block using
ESP_LOGI, WiFi.mode, _wifiDisabledByEthernet) to first check the current WiFi
mode/status (e.g., WiFi.getMode() != WIFI_OFF or WiFi.status() == WL_CONNECTED)
and only call WiFi.mode(WIFI_OFF) and set _wifiDisabledByEthernet = true when
WiFi was actually on; leave the re-enable branch logic (which checks
_wifiDisabledByEthernet) unchanged so WiFi is only re-enabled if we truly
disabled it.
In `@src/MoonBase/SharedWebSocketServer.h`:
- Around line 111-114: Remove the unreachable NULL check on data returned from
buffer.c_str() to satisfy lint: delete the "!data" branch and keep the existing
empty-buffer guard (i.e., retain the check for buffer.length() == 0 or use
buffer.empty()), and remove the preceding comment that incorrectly suggests
String::c_str() can return NULL; update the block around the data and buffer
variables so only the empty check is performed.
In `@src/MoonLight/Modules/ModuleDrivers.h`:
- Around line 67-78: The readPins() update handler currently disables UART0 and
suppresses all esp logging when it sees GPIO 1 or 3 used as an LED pin (calls
Serial.end() and esp_log_set_vprintf(...) to noop), but it never restores UART
or the original vprintf when those pins are later freed; modify the handler to
track whether suppression was applied (e.g., a static or module-level bool like
uartSuppressed) and on each invocation check the current layerP.ledPins: if any
pin is 1 or 3 and uartSuppressed is false, perform the existing suppression and
set uartSuppressed=true; if no pin is 1 or 3 and uartSuppressed is true, call
Serial.begin(...) with the original baud (or reinitialize Serial appropriately)
and restore the previous esp_log_vprintf function (store the original vprintf
when suppressing), then set uartSuppressed=false so logging and serial are
re-enabled when GPIO1/3 are no longer used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7eb415cb-b6f4-413b-93e7-4620b74dc22d
📒 Files selected for processing (4)
lib/framework/EthernetSettingsService.cpplib/framework/EthernetSettingsService.hsrc/MoonBase/SharedWebSocketServer.hsrc/MoonLight/Modules/ModuleDrivers.h
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
backend ======= - ethernet Settings: WiFi off only if not off - Shared WebSocket: transmitData remove !data - Drivers: tweak log surpression if pin 1 or 3 used - Module IO: simplify default SDA/SCL/TX/RX assignment
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/MoonLight/Modules/ModuleDrivers.h (1)
78-85: Consider parameterizing the serial baud rate instead of hardcoding 115200.Line 84 restores
Serialat a hardcoded115200baud. Currently,SERIAL_BAUD_RATEis defined as115200inmain.cpp, so this matches. However, the restore logic in a header file cannot reference themain.cppconstant. IfSERIAL_BAUD_RATEis later changed, the restore path will silently use the wrong baud rate. Pass the baud rate as a constructor parameter toModuleDriversand store it as a member variable, or centralize the constant in a shared header that both files can include.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/MoonLight/Modules/ModuleDrivers.h` around lines 78 - 85, The restore path hardcodes Serial.begin(115200) causing mismatch if SERIAL_BAUD_RATE changes; modify ModuleDrivers to accept a baudRate (or use a shared constant) and store it as a member (e.g., _serialBaudRate) in the ModuleDrivers class, initialize it via the ModuleDrivers constructor, and replace the hardcoded 115200 in the suppression/restore logic (the branch that checks needsUartSuppression/_uartSuppressed and calls Serial.begin) to use _serialBaudRate instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/framework/EthernetSettingsService.cpp`:
- Around line 135-140: EthernetSettingsService re-enables WiFi using
WiFi.begin() without credentials, which fails when WiFi.persistent(false)
cleared in WiFiSettingsService; modify EthernetSettingsService to either call
WiFiSettingsService::reconfigureWiFiConnection() or obtain SSID/password before
calling WiFi.begin(): add a WiFiSettingsService reference to
EthernetSettingsService (constructor + member) and replace the
WiFi.mode/WiFi.begin sequence inside the _wifiDisabledByEthernet branch (where
_wifiDisabledByEthernet is cleared) with a call to
wifiSettingsService->reconfigureWiFiConnection() (or, if you choose the simpler
path, cache SSID/password in EthernetSettingsService and pass them into
WiFi.begin(ssid, password)), and ensure access is synchronized to avoid
concurrent WiFi state changes.
In `@src/MoonBase/SharedWebSocketServer.h`:
- Around line 112-117: The local String buffer in transmitData() is destroyed
when the function returns, causing dangling pointers passed to
client->sendMessage(data) and _handler.sendAll(data); change the stack-local
declaration to a static String (e.g., replace "String buffer;" with "static
String buffer;") and ensure you call buffer.clear() before populating it for
reuse, or alternatively switch to a reference-counted/persistent container so
the data remains valid until the async send completes; update the code paths
around transmitData(), client->sendMessage, and _handler.sendAll accordingly.
---
Nitpick comments:
In `@src/MoonLight/Modules/ModuleDrivers.h`:
- Around line 78-85: The restore path hardcodes Serial.begin(115200) causing
mismatch if SERIAL_BAUD_RATE changes; modify ModuleDrivers to accept a baudRate
(or use a shared constant) and store it as a member (e.g., _serialBaudRate) in
the ModuleDrivers class, initialize it via the ModuleDrivers constructor, and
replace the hardcoded 115200 in the suppression/restore logic (the branch that
checks needsUartSuppression/_uartSuppressed and calls Serial.begin) to use
_serialBaudRate instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 29a37082-bd88-426f-ab80-c91c1009efc4
📒 Files selected for processing (4)
lib/framework/EthernetSettingsService.cppsrc/MoonBase/Modules/ModuleIO.hsrc/MoonBase/SharedWebSocketServer.hsrc/MoonLight/Modules/ModuleDrivers.h
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
docs ==== - inputoutput: add GPIO tx/rx used for led output backend ======= - Ethernet settings: Ethernet lost, let WiFi service reconnect - WiFi settings : manageSTA check on ETH.connected
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/framework/WiFiSettingsService.cpp (1)
320-331:⚠️ Potential issue | 🔴 CriticalRemove the orphaned
elseclause that breaks compilation whenFT_ETHERNETis disabled.When the
FT_ETHERNETfeature flag is disabled, the preprocessor removes lines 320–325 (the#ifblock), leaving a bareelseat line 326 with no precedingifstatement. This causes a syntax error in non-Ethernet builds.Fix
void WiFiSettingsService::manageSTA() { // Abort if already connected, if we have no SSID, or are in offline mode if (WiFi.isConnected() || _state.wifiSettings.empty() || _state.staConnectionMode == (u_int8_t)STAConnectionMode::OFFLINE) { return; } // 🌙 Don't reconnect WiFi while ethernet is active — saves ~50KB heap on ESP32-D0 `#if` FT_ENABLED(FT_ETHERNET) if (ETH.connected()) { return; } `#endif` - else- { `#ifdef` SERIAL_INFO - Serial.println("Connecting to WiFi...");+ Serial.println("Connecting to WiFi..."); `#endif` - connectToWiFi();- }+ connectToWiFi(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/framework/WiFiSettingsService.cpp` around lines 320 - 331, The orphaned `else` left by the FT_ETHERNET preprocessor block causes a compile error when FT_ETHERNET is disabled; update the logic around the FT_ENABLED(FT_ETHERNET) / ETH.connected() check so there is no standalone `else`: keep the early return when ETH.connected() is true (inside the FT_ENABLED block) and move the Serial.println("Connecting to WiFi...") and connectToWiFi() call to run unconditionally after that block (or guarded by `#else/`#endif properly) so connectToWiFi() executes only when we didn't return from ETH.connected(); remove the orphaned `else` token and ensure SERIAL_INFO and Serial.println usage and the call to connectToWiFi() remain reachable in non-Ethernet builds.
🧹 Nitpick comments (1)
lib/framework/EthernetSettingsService.cpp (1)
43-46: Use the repo tag constant in the new Ethernet logs.These additions still log with
SVK_TAG. Please switch them to the appropriateML_TAG/MB_TAGconstant for this layer.As per coding guidelines "Use tag constants
ML_TAGfor MoonLight logging andMB_TAGfor MoonBase logging".Also applies to: 133-136, 142-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/framework/EthernetSettingsService.cpp` around lines 43 - 46, Replace the hardcoded SVK_TAG used in the new Ethernet-related ESP_LOGI calls with the layer-appropriate repo tag constant (use ML_TAG for MoonLight-layer code in EthernetSettingsService.cpp); locate the ESP_LOGI(...) calls that currently pass SVK_TAG (including the ones around the WiFi disable/heap messages and the other occurrences you modified) and swap SVK_TAG to ML_TAG (or MB_TAG if this file is part of MoonBase) so logging follows the ML_TAG/MB_TAG convention; ensure all modified log lines consistently use the chosen tag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/framework/EthernetSettingsService.cpp`:
- Around line 137-143: The branch that clears _wifiDisabledByEthernet must also
kick the WiFi reconnection path immediately instead of waiting for
WIFI_RECONNECTION_DELAY; after setting _wifiDisabledByEthernet = false, call or
signal the WiFiSettingsService to run its reconnect logic right away (e.g.,
invoke WiFiSettingsService::manageSTA() or a public method like
WiFiSettingsService::requestImmediateReconnect()/triggerImmediateReconnection())
so WiFiSettingsService::loop() doesn't wait for the periodic throttle to attempt
reconnection.
---
Outside diff comments:
In `@lib/framework/WiFiSettingsService.cpp`:
- Around line 320-331: The orphaned `else` left by the FT_ETHERNET preprocessor
block causes a compile error when FT_ETHERNET is disabled; update the logic
around the FT_ENABLED(FT_ETHERNET) / ETH.connected() check so there is no
standalone `else`: keep the early return when ETH.connected() is true (inside
the FT_ENABLED block) and move the Serial.println("Connecting to WiFi...") and
connectToWiFi() call to run unconditionally after that block (or guarded by
`#else/`#endif properly) so connectToWiFi() executes only when we didn't return
from ETH.connected(); remove the orphaned `else` token and ensure SERIAL_INFO
and Serial.println usage and the call to connectToWiFi() remain reachable in
non-Ethernet builds.
---
Nitpick comments:
In `@lib/framework/EthernetSettingsService.cpp`:
- Around line 43-46: Replace the hardcoded SVK_TAG used in the new
Ethernet-related ESP_LOGI calls with the layer-appropriate repo tag constant
(use ML_TAG for MoonLight-layer code in EthernetSettingsService.cpp); locate the
ESP_LOGI(...) calls that currently pass SVK_TAG (including the ones around the
WiFi disable/heap messages and the other occurrences you modified) and swap
SVK_TAG to ML_TAG (or MB_TAG if this file is part of MoonBase) so logging
follows the ML_TAG/MB_TAG convention; ensure all modified log lines consistently
use the chosen tag.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3425e1b2-ad06-458d-ae0b-313bbcaa9dd9
📒 Files selected for processing (4)
docs/moonbase/inputoutput.mdlib/framework/EthernetSettingsService.cpplib/framework/WiFiSettingsService.cppsrc/MoonLight/Modules/ModuleDrivers.h
Uh oh!
There was an error while loading. Please reload this page.
ewowi
commented
Mar 16, 2026
CollaboratorAuthor
@coderabbitai, I am about to merge this PR, please do an in-depth review of all the commits made in this PR + commit 59f0149 (by mistake made this commit against main instead of in this PR), make a summary, a recommendation to merge and a list of possible future actions. |
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll gather the full commit list and diffs for PR 🏁 Script executed: Length of output: 1550 🏁 Script executed: Length of output: 41533
📋 Review SummaryStray commit |
| ✅ | All ethernet variables are now public and cross-platform with safe -1 defaults |
| ✅ | #if CONFIG_ETH_USE_ESP32_EMAC gate correctly isolates RMII-only variables |
v_ETH_PHY_ADDR is not reset in the reset block at the top of the FT_ETHERNET section in readPins(). W5500 sets it to 1; LAN8720 never sets it — it relies on the class header default of 0. If a user changes from W5500 → LAN8720 without rebooting, v_ETH_PHY_ADDR stays stale at 1, causing LAN8720 to try the wrong PHY address. Fix: add ess->v_ETH_PHY_ADDR = 0; to the reset block. |
lib/framework/EthernetSettingsService.cpp — configureNetwork()
| ✅ | SPI and RMII branches are cleanly separated |
The else { ETH.begin(); }fallback executes even when ethernetType == eth_None. When neither flag is set (i.e. no ethernet configured), ETH.begin() is still called with default arguments. This is almost certainly unintentional and could cause a crash or spurious network initialisation on boards with no ethernet hardware. Fix: guard this with else if (/* some explicit fallback condition */) or remove it entirely. |
src/MoonBase/Modules/ModuleIO.h — readPins()
| ✅ | New IO_EthernetTypeEnum (None/LAN8720/W5500) + ethernetType select control is clean |
| ✅ | Dig-Octa RMII pin assignment (MDC=23, MDIO=18, CLK=17, RMII data pins 19/21/22/25/26/27 reserved) matches the QuinLED Dig-Octa-32-8L pinout |
| ✅ | newBoardID → _newBoardPreset, _current_board_preset → _currentBoardPreset (naming hygiene) |
assignIfValid lambda removed — SPI pin assignments (v_ETH_SPI_SCK, v_ETH_SPI_MISO, v_ETH_SPI_MOSI, v_ETH_PHY_CS, v_ETH_PHY_IRQ) are now made directly without GPIO_IS_VALID_GPIO() validation. For RMII data pins this is fine (hardwired in silicon), but for W5500 SPI pins that are user-configurable it is a regression — an invalid GPIO number could be assigned silently. | |
| ℹ️ | ess->initEthernet() is called from readPins() on every board-preset reconfiguration. On boot this means a double-init sequence (once at startup, once when the preset fires). This works today but is worth tracking. |
platformio.ini
| ✅ | FastLED updated from 34ba2f75 (20260312) → 97ab2971 (20260315); old pinned hashes left as commented history — good practice |
src/MoonLight/Modules/ModuleEffects.h
| ✅ | Whitespace/alignment only — no functional change |
PR commit 51da16ac — "WiFi off/on when Ethernit on/off"
lib/framework/EthernetSettingsService.cpp + .h
| ✅ | _wifiDisabledByEthernet flag correctly tracks whether this service disabled WiFi, preventing spurious re-enable on unrelated events |
| ✅ | Forcing WIFI_OFF before initEthernet() with heap logging is a good defensive measure for ESP32-D0's constrained heap |
| ✅ | Caching ETH.connected() into ethConnected avoids repeated driver calls in updateEthernet() |
| ℹ️ | When ethernet disconnects and _wifiDisabledByEthernet is cleared, reconnection is delegated to WiFiSettingsService. This is correct by design but relies on WiFiSettingsService::manageSTA() firing in time — see commit f340dca0 below |
PR commit ac6b8010 — "Ethernet and IO tweaks"
src/MoonBase/Modules/ModuleIO.h — I2C pin assignment
| ✅ | Replaces the long per-target #ifdef chain with SDA, SCL, TX, RX constants from pins_arduino.h — significantly reduces maintenance burden for new targets |
| ✅ | TX/RX (pin_Serial_TX, pin_Serial_RX) now assigned from board variant headers — portable |
src/MoonBase/SharedWebSocketServer.h
| ✅ | Empty-buffer early-return prevents sending a zero-length message to clients |
| ✅ | Caching buffer.c_str() as const char* data avoids redundant string operations in both send paths |
src/MoonLight/Modules/ModuleDrivers.h
| ✅ | UART0 TX/RX suppression when GPIO 1/3 are assigned as LED outputs is a practical workaround for Dig-Octa-style wiring. Capturing _origVprintf and restoring on exit is clean |
| ✅ | _moduleLightsControl and _moduleIO moved from public to private — good encapsulation |
PR commit f340dca0 — "re-disable WiFi while ethernet, Don't reconnect WiFi while ethernet"
lib/framework/WiFiSettingsService.cpp
| ✅ | Early-return in manageSTA() when ETH.connected() is true prevents WiFi from trying to reconnect while ethernet is active — important heap saving on ESP32-D0 |
| ✅ | Correctly guarded with FT_ENABLED(FT_ETHERNET) |
| ℹ️ | Edge case worth noting: if both ethernet and WiFi drop simultaneously (e.g. power cycle of the ethernet switch) and _wifiDisabledByEthernet was true, the flag will be cleared in updateEthernet() but manageSTA() will not see ETH.connected() → true, so WiFi reconnection should proceed. This appears to work correctly but is worth an integration test. |
docs/moonbase/inputoutput.md
| ✅ | Full rewrite from bullet list to structured reference documentation with tables — excellent quality improvement |
| ✅ | Board table, pin-types table, ethernet type table, and UART0/GPIO 1+3 warning block are all accurate and well-written |
✅ Recommendation: Merge with follow-up issues
The overall design is solid. The cross-platform ethernet architecture is a meaningful improvement over the old S3-only #ifdef approach, WiFi/Ethernet coexistence is handled correctly for the target hardware (ESP32-D0 heap constraints), and the documentation is now genuinely useful. No blocking issues found. The two known bugs noted in long-term memory and the ETH.begin() fallback issue are real but are non-crashing in the current hardware configurations (Dig-Octa never switches away from LAN8720; SE16/LightCrafter16 don't swap PHY types at runtime).
🔭 Possible future actions (prioritised)
| Priority | Area | Action |
|---|---|---|
| 🔴 High | ModuleIO.hreadPins() | Add ess->v_ETH_PHY_ADDR = 0; to the ethernet variable reset block to fix the W5500 → LAN8720 stale-address bug |
| 🔴 High | EthernetSettingsService.cppconfigureNetwork() | Guard the else { ETH.begin(); } fallback — it fires unconditionally when ethernetType == eth_None, which is wrong; remove or wrap in an explicit condition |
| 🟠 Medium | ModuleIO.hreadPins() | Re-add GPIO validity checking (equivalent to the removed assignIfValid lambda) for W5500 SPI pins to catch misconfigured boards early |
| 🟠 Medium | EthernetSettingsService | Integration test: simulate W5500 → LAN8720 switch at runtime (without reboot) and verify correct PHY address |
| 🟡 Low | WiFiSettingsService.cpp | Integration test: simultaneous ethernet + WiFi drop — verify WiFi reconnects as expected |
| 🟡 Low | ModuleDrivers.h | Document the UART0 suppression lifetime — currently it lasts for the lifetime of the readPins() result; consider re-evaluating if the user reconfigures pins back to non-LED usage |
| 🟡 Low | platformio.ini | Prune old commented-out FastLED hashes periodically to reduce noise |
| 🟢 Nice-to-have | ModuleIO.h | Consider adding a eth_ENC28J60 entry to IO_EthernetTypeEnum for future SPI ethernet expansion |
| 🟢 Nice-to-have | EthernetSettingsService | Add a v_ETH_PHY_RST reset to the reset block (it is reset to -1 already, but making it explicit alongside the others improves readability) |
Would you like me to open GitHub issues for the high-priority items?
✏️ Learnings added
Learnt from: ewowi
URL: https://github.com/MoonModules/MoonLight/pull/151
Timestamp: 2026-03-16T22:59:40.564Z
Learning: In MoonModules/MoonLight EthernetSettingsService.cpp configureNetwork(): There is an `else { ETH.begin(); }` fallback that executes even when ethernetType == eth_None (i.e. neither v_ETH_SPI_CONFIGURED nor v_ETH_RMII_CONFIGURED is true). This should be removed or guarded to avoid spurious ETH.begin() calls on boards with no ethernet hardware.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-03-16T14:28:21.568Z
Learning: In MoonModules/MoonLight EthernetSettingsService: v_ETH_PHY_ADDR is NOT reset in readPins() before the ethType switch. W5500 explicitly sets it to 1; LAN8720 relies on the header default of 0. This is a latent bug if readPins() is called after a W5500 configuration and then LAN8720 is selected — addr would be stale at 1. Fix: add `ess->v_ETH_PHY_ADDR = 0;` to the ethernet variable reset block at the top of the FT_ETHERNET section in readPins().
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2026-03-16T14:28:21.568Z
Learning: In MoonModules/MoonLight ModuleIO.h readPins(): The assignIfValid lambda (which called GPIO_IS_VALID_GPIO() before assigning SPI ethernet pin variables) was removed in the ethernet rework commit 59f0149. SPI pin assignments (pin_SPI_SCK, pin_SPI_MISO, pin_SPI_MOSI, pin_PHY_CS, pin_PHY_IRQ) are now assigned directly without GPIO validity checking. For RMII pins this is fine (hardware-fixed), but for user-configurable W5500 SPI pins this is a regression.
ewowi
commented
Mar 16, 2026
CollaboratorAuthor
@coderabbitai, regarding follow up I will submit another commit:
Not fixed (verified, no action needed):
|
Tip For best results, initiate chat on the files or code changes.
The PR is clear to merge. 🎉 ✏️ Learnings added
⛔ Learnings removed🧠 Learnings usedIf you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
Refactor
Documentation