Skip to content

[RF] Fix CC1101 not initializing on boot when config exists#2268

Closed
1technophile wants to merge 1 commit intodevelopmentfrom
fix/cc1101-init-on-boot
Closed

[RF] Fix CC1101 not initializing on boot when config exists#2268
1technophile wants to merge 1 commit intodevelopmentfrom
fix/cc1101-init-on-boot

Conversation

@1technophile
Copy link
Owner

Fixes #2264

When RF configuration was saved to NVS, the CC1101 module would not initialize properly on boot, requiring a manual toggle between RTL_433 and RF modes in the WebUI to activate it.

Root cause: The loadFromStorage() method in RFConfiguration.cpp loaded the saved configuration but failed to call iRFReceiver.enable() when config existed. This only affected users with saved configurations.

The "no config found" path correctly called enable(), and the loadFromMessage() method (used by WebUI) also called disable()/enable(), explaining why the WebUI workaround worked.

Solution: Add iRFReceiver.enable() after loading configuration in loadFromStorage() to match the behavior of both the "no config" branch and the loadFromMessage() method.

This ensures both code paths (config exists vs. doesn't exist) behave identically: load config, then enable receiver.

🤖 Generated with Claude Code

Description:

Checklist:

  • The pull request is done against the latest development branch
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • I accept the DCO.

Fixes #2264

When RF configuration was saved to NVS, the CC1101 module would not
initialize properly on boot, requiring a manual toggle between RTL_433
and RF modes in the WebUI to activate it.

Root cause: The loadFromStorage() method in RFConfiguration.cpp loaded
the saved configuration but failed to call iRFReceiver.enable() when
config existed. This only affected users with saved configurations.

The "no config found" path correctly called enable(), and the
loadFromMessage() method (used by WebUI) also called disable()/enable(),
explaining why the WebUI workaround worked.

Solution: Add iRFReceiver.enable() after loading configuration in
loadFromStorage() to match the behavior of both the "no config" branch
and the loadFromMessage() method.

This ensures both code paths (config exists vs. doesn't exist) behave
identically: load config, then enable receiver.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@claude
Copy link

claude bot commented Jan 5, 2026

Code Review: PR #2268 - Fix CC1101 not initializing on boot when config exists

Summary

This PR addresses a critical bug where the CC1101 RF module fails to initialize properly on boot when a saved configuration exists in NVS. The fix is a simple one-line addition that ensures iRFReceiver.enable() is called after loading the configuration from storage.

Code Quality: ✅ EXCELLENT

Strengths:

  1. Minimal and Focused: The fix is a single line change that directly addresses the root cause
  2. Consistency: Ensures both code paths (config exists vs. doesn't exist) behave identically
  3. Well-Documented: The PR description clearly explains the problem, root cause, and solution
  4. Follows Existing Patterns: Matches the behavior of loadFromMessage() which calls disable()/enable()

Analysis:

Looking at main/rf/RFConfiguration.cpp:141-168, the fix makes perfect sense:

Before the fix:

  • Line 163: When no config exists → calls iRFReceiver.enable()
  • Line 159: When config existsmissing iRFReceiver.enable()

After the fix:

  • Both paths now call enable() consistently ✅

Comparison with loadFromMessage() (lines 203-204):

iRFReceiver.disable();
iRFReceiver.enable();

This method always reinitializes the receiver, explaining why the WebUI workaround worked.

Potential Issues: ⚠️ MINOR CONSIDERATION

Question: Should we call disable() before enable()?

Looking at line 203-204 in loadFromMessage(), there's a deliberate pattern of calling disable() before enable(). This ensures a clean reinitialization.

Current fix:

iRFReceiver.enable();

Alternative approach:

iRFReceiver.disable();
iRFReceiver.enable();

Considerations:

  1. The "no config found" path (line 163) only calls enable() without disable(), so the current fix maintains that pattern
  2. However, during boot with existing config, the receiver might be in an unknown state
  3. The loadFromMessage() pattern suggests a full reinit might be safer

Recommendation: The current fix is likely sufficient since:

  • It's called during initial setup (setupCommonRF() at line 92 in commonRF.cpp)
  • The receiver shouldn't be enabled yet at boot time
  • It maintains consistency with the "no config" branch

If issues persist, consider adding iRFReceiver.disable() before enable() to match the loadFromMessage() pattern.

Performance: ✅ NO CONCERNS

  • Single method call during boot initialization
  • No performance impact

Security: ✅ NO CONCERNS

  • No security implications
  • Only affects initialization logic

Test Coverage: ⚠️ TESTING RECOMMENDED

Manual Testing Needed:

  1. Primary test case: Verify CC1101 initializes correctly on boot with saved config
  2. Edge cases:
    • Fresh install (no saved config) - should continue working
    • After saving config via WebUI - should work on next boot
    • Multiple reboots with saved config - should consistently work

Suggested Test Plan:

1. Flash firmware to ESP32 with CC1101
2. Configure via WebUI and save settings
3. Reboot device
4. Verify CC1101 receives signals immediately after boot (check for "C1101 spi Connection OK" in logs)
5. Repeat reboot test 3-5 times to ensure consistency

Best Practices: ✅ EXCELLENT

Recommendations:

1. ✅ APPROVE AS-IS (Recommended)

The fix is correct and addresses the reported issue. It's minimal, focused, and consistent with one of the existing code paths.

2. 🔍 Optional Enhancement (For consideration)

If you want to be extra safe and match the loadFromMessage() pattern more closely, consider:

JsonObject jo = jsonBuffer.as<JsonObject>();
fromJson(jo);
Log.notice(F("RF Config loaded" CR));
iRFReceiver.disable();  // Add this line for consistency with loadFromMessage()
iRFReceiver.enable();

However, this is likely not necessary since the receiver shouldn't be active during initial boot.

Conclusion: ✅ APPROVED

This is a clean, well-documented fix for a real bug. The code change is minimal, correct, and addresses the root cause identified in issue #2264.

Rating: 9/10

  • Excellent problem diagnosis
  • Minimal, focused solution
  • Good documentation
  • Ready to merge

The only minor consideration is whether to add disable() before enable() for extra safety, but the current implementation is correct and sufficient.

Great work! 🎉

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.

Changes between 1.8.1 and development breaks CC1101 (rcswitch / smartrc-cc1101-driver) default behaviour

1 participant