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

[RF] Proposal fix for issue #2139 + Extra documentation #2157

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

Odyno
Copy link
Contributor

@Odyno Odyno commented Jan 29, 2025

Description:

This pull request proposes a fix for issue #2139 and includes several improvements and enhancements to the RF module.

Changes:

  1. Conditional MQTT Message Sending:
    • Introduced two #define directives to allow users to choose which MQTT message to send for each RF signal received. Users can uncomment the desired line to enable the corresponding functionality.
#define RF_on_HAS_as_DeviceTrigger //uncomment this line so as to create a Home Assistant device trigger for each RF signal received
//#define RF_on_HAS_as_MQTTSencor //uncomment this line so as to create a Home Assistant MQTT sensor for each RF signal received
  1. Refresh of announceDeviceTrigger Message:
  • Updated the announceDeviceTrigger message to align with Home Assistant specifications. This change does not affect functionality but ensures compliance with the latest standards.
  1. Reformatting and Additional Documentation:
  • Reformatted the RF module code for better readability and maintainability + functions documentation
  • Added extra documentation to clarify the functionality and usage of the RF module on Site
  1. Improved Logging for RF Handling:
  • Enhanced logging to provide more detailed insights and debugging information for RF signal handling.

Impact:

These changes improve the flexibility and usability of the RF module, allowing users to customize their Home Assistant integration more easily. With this implementaion can be a broken retroconpatibilty

The enhanced logging and documentation will aid in troubleshooting and understanding the RF module's behavior.

Base for Future Work

The idea is to move the configuration to Home Assistant using MQTT Selectors. This will allow users to enable or disable modules, set the current frequency, and choose the type of message to send, all through MQTT Selectors.

Please review the proposed changes and provide feedback. Thank you!

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.

@Odyno
Copy link
Contributor Author

Odyno commented Jan 29, 2025

There appears to be some confusion regarding how the RF module is announced on Home Assistant. I have created this pull request as a preview of the work I am doing to address issue #2139. I am currently reviewing the RF module to clean up the code and improve the documentation. My suggestion is to test these changes for a couple of days to see if we can further enhance the documentation and user interaction.

@Odyno Odyno changed the title Proposal for #2139 + Restile of announceDeviceTrigger message [RF] Proposal fix for issue #2139 + Extra documentation Jan 29, 2025
… and documentation for optional device information

Fix null pointer references in RFtoMQTTdiscovery and update logging levels for RF signal handling
Refactor RF to MQTT discovery functions for improved clarity and parameter handling
@Odyno Odyno marked this pull request as ready for review February 9, 2025 19:10
@Odyno
Copy link
Contributor Author

Odyno commented Feb 9, 2025

@1technophile and all other contributors, I have completed a review of the code and documentation for the RF module. Please feel free to provide any comments and let me know if you would like to merge it into the master branch. Thank you!

@1technophile
Copy link
Owner

Thanks for the PR, give me a few days to review it

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