Skip to content

Conversation

@andig
Copy link
Member

@andig andig commented Oct 24, 2025

No description provided.

@andig andig requested review from naltatis and premultiply October 24, 2025 15:15
@andig andig added devices Specific device support needs documentation Triggers issue creation in evcc-io/docs labels Oct 24, 2025
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `charger/sgready-relay.go:67` </location>
<code_context>
-
-// NewSgReadyBoost creates SG Ready charger with boost relay
-func NewSgReadyBoost(ctx context.Context, embed *embed, charger api.Charger) (*SgReady, error) {
-	modeS := func(mode int64) error {
-		switch mode {
-		case Dim:
</code_context>

<issue_to_address>
**issue (complexity):** Consider refactoring the modeS closure to compute enable flags first and then apply them uniformly to reduce duplicated logic.

```suggestion
The `modeS` closure can be simplified by first computing the desired enable flags, then doing two uniform enable/disable calls. This removes most of the duplicated switch branches:

```go
modeS := func(mode int64) error {
    var (
        enableBoost = false
        enableDim   = false
    )

    switch mode {
    case Dim:
        if dim == nil {
            return api.ErrNotAvailable
        }
        enableDim = true

    case Boost:
        enableBoost = true

    case Normal:
        // both false

    default:
        return fmt.Errorf("invalid sgready mode: %d", mode)
    }

    // apply to boost
    if err := boost.Enable(enableBoost); err != nil {
        return err
    }
    // apply to dim if present
    if dim != nil {
        if err := dim.Enable(enableDim); err != nil {
            return err
        }
    }
    return nil
}
```

This collapses all “disable other, then set desired” patterns into a single preamble and uniform dispatch.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devices Specific device support needs documentation Triggers issue creation in evcc-io/docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants