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

implements #93 #94 #102

Merged
merged 26 commits into from
Feb 6, 2025
Merged

implements #93 #94 #102

merged 26 commits into from
Feb 6, 2025

Conversation

Pho3niX90
Copy link
Owner

@Pho3niX90 Pho3niX90 commented Feb 4, 2025

User description

implements #93 #94
fixes #96 #103 #104


PR Type

Enhancement


Description

  • Added support for up to 6 charging/discharge slots in the Solis Modbus integration.

  • Introduced new sensor definitions for additional time slots in hybrid_sensors.py.

  • Updated time.py to include new time slot configurations.

  • Incremented integration version to 1.5.7 in manifest.json.


Changes walkthrough 📝

Relevant files
Enhancement
hybrid_sensors.py
Extend sensor definitions for additional time slots           

custom_components/solis_modbus/data/hybrid_sensors.py

  • Added sensor definitions for slots 4 and 5.
  • Adjusted reserve register definitions for consistency.
  • Extended support for additional time slots with detailed
    configurations.
  • +74/-2   
    time.py
    Add time slot configurations for slots 4 and 5                     

    custom_components/solis_modbus/time.py

  • Added time slot configurations for slots 4 and 5.
  • Enabled new time slot entities for charging and discharging.
  • +10/-0   
    Configuration changes
    manifest.json
    Increment integration version to 1.5.7                                     

    custom_components/solis_modbus/manifest.json

    • Updated integration version from 1.5.6 to 1.5.7.
    +1/-1     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copy link

    github-actions bot commented Feb 4, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis 🔶

    94 - Partially compliant

    Compliant requirements:

    • Add support for 6 charging/discharge slots with different SOC and Current for each slot in the latest Solis firmware.

    Non-compliant requirements:

    []

    Requires further human verification:

    []

    93 - Partially compliant

    Compliant requirements:

    • Ensure "Time Of Use" charging works correctly for the S6-EH3P20K-H Hybrid inverter.

    Non-compliant requirements:

    []

    Requires further human verification:

    []

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Possible Redundancy

    The addition of multiple "reserve" registers (e.g., ['43051', '43052'], ['43061', '43062']) should be reviewed to ensure they are necessary and do not introduce redundancy or unused code.

    {"type": "reserve", "register": ['43051', '43052']},
    
    {"type": "SS", "name": "Solis Time-Charging Charge Start Hour (Slot 2)",
     "unique": "solis_modbus_inverter_time_charging_start_hour_slot2", "register": ['43153'],
     "multiplier": 0, "unit_of_measurement": UnitOfTime.HOURS, "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Charge Start Minute (Slot 2)",
     "unique": "solis_modbus_inverter_time_charging_start_minute_slot2", "register": ['43154'],
     "multiplier": 0, "unit_of_measurement": UnitOfTime.MINUTES,
     "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Charge End Hour (Slot 2)",
     "unique": "solis_modbus_inverter_time_charging_end_hour_slot2", "register": ['43155'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.HOURS, "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Charge End Minute (Slot 2)",
     "unique": "solis_modbus_inverter_time_charging_end_minute_slot2",
     "register": ['43156'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.MINUTES, "state_class": SensorStateClass.MEASUREMENT},
    
    {"type": "SS", "name": "Solis Time-Charging Discharge Start Hour (Slot 2)",
     "unique": "solis_modbus_inverter_time_discharge_start_hour_slot2",
     "register": ['43157'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.HOURS, "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Discharge Start Minute (Slot 2)",
     "unique": "solis_modbus_inverter_time_discharge_start_minute_slot2",
     "register": ['43158'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.MINUTES, "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Discharge End Hour (Slot 2)",
     "unique": "solis_modbus_inverter_time_discharge_end_hour_slot2",
     "register": ['43159'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.HOURS, "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Discharge End Minute (Slot 2)",
     "unique": "solis_modbus_inverter_time_discharge_end_minute_slot2",
     "register": ['43160'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.MINUTES, "state_class": SensorStateClass.MEASUREMENT},
    
    {"type": "reserve", "register": ['43061', '43062']},
    
    {"type": "SS", "name": "Solis Time-Charging Charge Start Hour (Slot 3)",
     "unique": "solis_modbus_inverter_time_charging_start_hour_slot3",
     "register": ['43163'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.HOURS, "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Charge Start Minute (Slot 3)",
     "unique": "solis_modbus_inverter_time_charging_start_minute_slot3",
     "register": ['43164'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.MINUTES, "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Charge End Hour (Slot 3)",
     "unique": "solis_modbus_inverter_time_charging_end_hour_slot3",
     "register": ['43165'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.HOURS, "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Charge End Minute (Slot 3)",
     "unique": "solis_modbus_inverter_time_charging_end_minute_slot3",
     "register": ['43166'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.MINUTES, "state_class": SensorStateClass.MEASUREMENT},
    
    {"type": "SS", "name": "Solis Time-Charging Discharge Start Hour (Slot 3)",
     "unique": "solis_modbus_inverter_time_discharge_start_hour_slot3",
     "register": ['43167'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.HOURS, "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Discharge Start Minute (Slot 3)",
     "unique": "solis_modbus_inverter_time_discharge_start_minute_slot3",
     "register": ['43168'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.MINUTES, "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Discharge End Hour (Slot 3)",
     "unique": "solis_modbus_inverter_time_discharge_end_hour_slot3",
     "register": ['43169'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.HOURS, "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Discharge End Minute (Slot 3)",
     "unique": "solis_modbus_inverter_time_discharge_end_minute_slot3",
     "register": ['43170'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.MINUTES, "state_class": SensorStateClass.MEASUREMENT},
    
    {"type": "reserve", "register": ['43071', '43072']},
    
    {"type": "SS", "name": "Solis Time-Charging Charge Start Hour (Slot 4)",
     "unique": "solis_modbus_inverter_time_charging_start_hour_slot4",
     "register": ['43173'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.HOURS, "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Charge Start Minute (Slot 4)",
     "unique": "solis_modbus_inverter_time_charging_start_minute_slot4",
     "register": ['43174'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.MINUTES, "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Charge End Hour (Slot 4)",
     "unique": "solis_modbus_inverter_time_charging_end_hour_slot4",
     "register": ['43175'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.HOURS, "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Charge End Minute (Slot 4)",
     "unique": "solis_modbus_inverter_time_charging_end_minute_slot4",
     "register": ['43176'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.MINUTES, "state_class": SensorStateClass.MEASUREMENT},
    
    {"type": "SS", "name": "Solis Time-Charging Discharge Start Hour (Slot 4)",
     "unique": "solis_modbus_inverter_time_discharge_start_hour_slot4",
     "register": ['43177'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.HOURS, "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Discharge Start Minute (Slot 4)",
     "unique": "solis_modbus_inverter_time_discharge_start_minute_slot4",
     "register": ['43178'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.MINUTES, "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Discharge End Hour (Slot 4)",
     "unique": "solis_modbus_inverter_time_discharge_end_hour_slot4",
     "register": ['43179'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.HOURS, "state_class": SensorStateClass.MEASUREMENT},
    {"type": "SS", "name": "Solis Time-Charging Discharge End Minute (Slot 4)",
     "unique": "solis_modbus_inverter_time_discharge_end_minute_slot4",
     "register": ['43180'], "multiplier": 0,
     "unit_of_measurement": UnitOfTime.MINUTES, "state_class": SensorStateClass.MEASUREMENT},
    
    {"type": "reserve", "register": ['43081', '43082']},
    Duplicate Entries

    There appear to be duplicate entries for Slot 5 in the timeent list (lines 65-73). This should be reviewed to confirm if it is intentional or an oversight.

    {"type": "STE", "name": "Solis Time-Charging Charge Start (Slot 5)", "register": 43173, "enabled": True},
    {"type": "STE", "name": "Solis Time-Charging Charge End (Slot 5)", "register": 43175, "enabled": True},
    {"type": "STE", "name": "Solis Time-Charging Discharge Start (Slot 5)", "register": 43177, "enabled": True},
    {"type": "STE", "name": "Solis Time-Charging Discharge End (Slot 5)", "register": 43179, "enabled": True},
    
    {"type": "STE", "name": "Solis Time-Charging Charge Start (Slot 5)", "register": 43183, "enabled": True},
    {"type": "STE", "name": "Solis Time-Charging Charge End (Slot 5)", "register": 43185, "enabled": True},
    {"type": "STE", "name": "Solis Time-Charging Discharge Start (Slot 5)", "register": 43187, "enabled": True},
    {"type": "STE", "name": "Solis Time-Charging Discharge End (Slot 5)", "register": 43189, "enabled": True},

    Copy link

    github-actions bot commented Feb 4, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Remove duplicate "Slot 5" entries

    Remove duplicate entries for "Slot 5" in the timeent list to prevent redundant
    processing or potential conflicts during runtime.

    custom_components/solis_modbus/time.py [65-73]

     {"type": "STE", "name": "Solis Time-Charging Charge Start (Slot 5)", "register": 43173, "enabled": True},
    -{"type": "STE", "name": "Solis Time-Charging Charge Start (Slot 5)", "register": 43183, "enabled": True},
    Suggestion importance[1-10]: 9

    Why: Removing duplicate entries for "Slot 5" prevents redundant processing and potential runtime conflicts, addressing a clear issue in the PR. This is a critical fix for correctness.

    9
    General
    Add metadata for "reserve" entries

    Ensure that the "reserve" entries with register values like ['43051', '43052'] and
    others are properly validated to avoid potential misconfiguration or runtime errors,
    as they lack additional metadata like "name" or "unique".

    custom_components/solis_modbus/data/hybrid_sensors.py [525]

    -{"type": "reserve", "register": ['43051', '43052']},
    +{"type": "reserve", "register": ['43051', '43052'], "name": "Reserve Slot 1", "unique": "solis_modbus_reserve_slot1"},
    Suggestion importance[1-10]: 7

    Why: Adding metadata like "name" and "unique" for "reserve" entries improves clarity and maintainability, reducing the risk of misconfiguration. However, the suggestion assumes the metadata format without confirming its necessity in the context of the PR.

    7
    Ensure unique "reserve" register values

    Verify that the register values for "reserve" entries are unique and do not overlap
    with other entries to avoid potential conflicts in register mapping.

    custom_components/solis_modbus/data/hybrid_sensors.py [559]

    -{"type": "reserve", "register": ['43061', '43062']},
    +{"type": "reserve", "register": ['43061', '43062'], "name": "Reserve Slot 2", "unique": "solis_modbus_reserve_slot2"},
    Suggestion importance[1-10]: 6

    Why: Ensuring unique register values for "reserve" entries is a valid suggestion to avoid conflicts. However, the suggestion's improved code assumes the addition of metadata without confirming its necessity or relevance in the PR context.

    6

    …ge_slots
    
    # Conflicts:
    #	custom_components/solis_modbus/data/hybrid_sensors.py
    #	custom_components/solis_modbus/manifest.json
    #	custom_components/solis_modbus/modbus_controller.py
    fix: tou UOM
    fix: reduce log spam when attempting reconnections
    @Pho3niX90 Pho3niX90 mentioned this pull request Feb 6, 2025
    @Pho3niX90 Pho3niX90 mentioned this pull request Feb 6, 2025
    @Pho3niX90 Pho3niX90 merged commit 61561dd into master Feb 6, 2025
    7 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    Cannot connect to Datalogger
    1 participant