Add Enovates charger - #30477
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
CurrentPower, the register is documented asint16but is decoded viars485.RTUInt16ToFloat64, which will mis-handle negative values; consider using a signed conversion (e.g. cast viaint16(binary.BigEndian.Uint16(b))) to reflect the documented type. - The
Statusmethod passesstring(b)directly toapi.ChargeStatusString, which will include any NUL padding from the two registers; trimming trailing\x00(similar totrimModbusString) before conversion would make the status decoding more robust. - In
MaxCurrentMillis,curr := uint16(current * 1e3)will silently wrap for negative or overly large currents; adding bounds checking (e.g. clamp to [0, math.MaxUint16] or the charger’s max register value) would prevent unintended values being written to the EMS limit register.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `CurrentPower`, the register is documented as `int16` but is decoded via `rs485.RTUInt16ToFloat64`, which will mis-handle negative values; consider using a signed conversion (e.g. cast via `int16(binary.BigEndian.Uint16(b))`) to reflect the documented type.
- The `Status` method passes `string(b)` directly to `api.ChargeStatusString`, which will include any NUL padding from the two registers; trimming trailing `\x00` (similar to `trimModbusString`) before conversion would make the status decoding more robust.
- In `MaxCurrentMillis`, `curr := uint16(current * 1e3)` will silently wrap for negative or overly large currents; adding bounds checking (e.g. clamp to [0, math.MaxUint16] or the charger’s max register value) would prevent unintended values being written to the EMS limit register.
## Individual Comments
### Comment 1
<location path="charger/enovates.go" line_range="152-153" />
<code_context>
+var _ api.ChargerEx = (*Enovates)(nil)
+
+// MaxCurrentMillis implements the api.ChargerEx interface
+func (wb *Enovates) MaxCurrentMillis(current float64) error {
+ curr := uint16(current * 1e3)
+
+ err := wb.setCurrent(curr)
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against negative or excessively large `current` values before converting to `uint16`.
Casting `current * 1e3` directly to `uint16` causes negative values to wrap and large values to overflow silently. Please validate or clamp the value before conversion (e.g. `<= 0` → 0, `> maxSupported` → max) or return an error on out-of-range input to avoid unexpected EMS limits at the charger.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| func (wb *Enovates) MaxCurrentMillis(current float64) error { | ||
| curr := uint16(current * 1e3) |
There was a problem hiding this comment.
issue (bug_risk): Guard against negative or excessively large current values before converting to uint16.
Casting current * 1e3 directly to uint16 causes negative values to wrap and large values to overflow silently. Please validate or clamp the value before conversion (e.g. <= 0 → 0, > maxSupported → max) or return an error on out-of-range input to avoid unexpected EMS limits at the charger.
Co-authored-by: premultiply <4681172+premultiply@users.noreply.github.com>
|
Add rfid reading @copilot |
Implemented in |
The Identifier registration is unconditional, so a plain method plus interface assertion suffices. With no conditional capabilities left, drop the implement.Caps embed and implement.New().
|
Can't get this to work. But there is still no modbus server running on the CP. |
|
Then we're probably out of luck :( |
version=1.13.1.3, name=ChargePointControllerLight, groupID=com.enovates.lccl, artificatID=ChargePointControllerLight, buildNumber=1744126097163, buildDate=2025-04-08 15:28 +0000 I'm guessing the 'ChargePointControllerLight' means it's handicapped... I'll try to gain some info, but i just noticed Enovates filed for bankrupcy about a week. Hoping firmware updates get released but not counting on it. Thanks for your efforts though! |
|
Shell Recharge Advanced 2.1 no luck, Modbus is refused. have i missed something? |
You haven’t. The service is probably removed because the controller is too light to run it. Take the fakexemex route or use your car to change the amps if you can. |
|
Hey 👋 I made the HA integration and library. I may be able to help, but I must mention I do not (currently) represent Enovates. If you are on firmware 2.13 or later, the Modbus options should be there and working. Do not assume things based on the internal package versions/names, they are not meant for your eyes 😉. I will also drop this totally unrelated link here with a small word of warning: Only upgrade your firmware via the official channel provided by your CPO, otherwise you may lose all support or warranty. But isn't it suspicious that the fw_upgrade zip on that page has the word generic in the name? 🙈 |
|
@dries007 always happy to hand out tokens to new contributors! For this specific integration I'm lacking a clear picture what the status is. It seems it was never tested in the first place since even basic modbus TCP connectivity wasn't working. |
Fix #30476
Adds an Enovates charger via Modbus TCP, controlled through its EMS-over-Modbus current limit. Covers Enovates ENO ONE and its Shell Recharge "Home Advanced" rebrand, a common alternative to the Alfen-based Shell/NewMotion units. Register map follows the official Enovates Modbus integration; control writes the EMS limit register, status is derived from the IEC 61851 Mode 3 state.