Add current limit fallback for OCPP 1.6 chargers#1959
Open
lkiw wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR extends OCPP v1.6 charge rate control by introducing a configuration-based fallback mechanism. When Smart Charging profiles are unavailable or rejected, the system now attempts to set a ChangesConfiguration-based charge rate fallback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/test_set_charge_rate_v16.py (1)
151-175: ⚡ Quick winDuplicate test detected.
The test
test_smart_charging_not_supported_charge_rate_unknown_returns_false(lines 151-175) appears to duplicate the behavior of the modified testtest_smart_charging_not_supported_returns_false_no_notify(lines 90-120). Both tests verify the same scenario: SMART not supported, ChargeRate returns "Unknown", configure not called, returns False.Consider removing one of these tests to reduce maintenance burden.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_set_charge_rate_v16.py` around lines 151 - 175, Remove the duplicate test by deleting either test_smart_charging_not_supported_charge_rate_unknown_returns_false or test_smart_charging_not_supported_returns_false_no_notify so only one remains; both cover the same scenario (SMART not supported, cp_v16.get_configuration returns "Unknown", configure and call must not be invoked and set_charge_rate returns False). Keep the clearer or more fully named test (referenced by the test function names above) and ensure the remaining test still sets cp_v16._attr_supported_features = prof.NONE and monkeypatches get_configuration, configure, and call as in the diff so behavior is preserved.custom_components/ocpp/ocppv16.py (1)
589-592: 💤 Low valueConsider simplifying type conversion.
The conversion
int(float(limit_amps))is redundant whenlimit_ampsis already anintper the function signature. If this is defensive programming for unexpected input types, consider adding a docstring note. Otherwise,int(limit_amps)should suffice.♻️ Simplified conversion
try: - amps = int(float(limit_amps)) + amps = int(limit_amps) except (TypeError, ValueError): return False🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@custom_components/ocpp/ocppv16.py` around lines 589 - 592, The int(float(limit_amps)) conversion is redundant; replace it with amps = int(limit_amps) to avoid unnecessary float casting (keep the existing except (TypeError, ValueError): return False behavior). If the code must defensively accept non-int inputs, instead add a brief docstring or inline comment on the function handling limit_amps to explain why non-int strings might be passed and why the try/except is needed; reference the variable limit_amps and the amps assignment in the current try/except block when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@custom_components/ocpp/ocppv16.py`:
- Around line 589-592: The int(float(limit_amps)) conversion is redundant;
replace it with amps = int(limit_amps) to avoid unnecessary float casting (keep
the existing except (TypeError, ValueError): return False behavior). If the code
must defensively accept non-int inputs, instead add a brief docstring or inline
comment on the function handling limit_amps to explain why non-int strings might
be passed and why the try/except is needed; reference the variable limit_amps
and the amps assignment in the current try/except block when making the change.
In `@tests/test_set_charge_rate_v16.py`:
- Around line 151-175: Remove the duplicate test by deleting either
test_smart_charging_not_supported_charge_rate_unknown_returns_false or
test_smart_charging_not_supported_returns_false_no_notify so only one remains;
both cover the same scenario (SMART not supported, cp_v16.get_configuration
returns "Unknown", configure and call must not be invoked and set_charge_rate
returns False). Keep the clearer or more fully named test (referenced by the
test function names above) and ensure the remaining test still sets
cp_v16._attr_supported_features = prof.NONE and monkeypatches get_configuration,
configure, and call as in the diff so behavior is preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2d09d8a8-e68d-4cc3-b8fd-000e2d880132
📒 Files selected for processing (3)
custom_components/ocpp/ocppv16.pydocs/supported-devices.mdtests/test_set_charge_rate_v16.py
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 free
to 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.
This PR adds a small OCPP 1.6 current limit fallback for chargers that do not expose or accept standard smart charging profiles.
The existing
SetChargingProfileflow remains the primary path. If smart charging is unavailable or all standard profile attempts are rejected, the integration now tries an optional charger configuration key for current limiting when supported by the device.Changes
Summary by CodeRabbit
New Features
Documentation