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

Add Peblar #16451

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Add Peblar #16451

wants to merge 13 commits into from

Conversation

PieVo
Copy link

@PieVo PieVo commented Oct 2, 2024

Hey EVCC team,

Here is a charger implementation for the Peblar chargers (https://peblar.com/products/ev-charging). Developer info on the modbus API is located at developer.peblar.com.

I've been using this implementation for a while and tested it for the 1p3p switchover to work. This switchover won't work on all variants due to relay configurations (and 1 phase connected systems ofc).

I've checked the doc build to run fine and cleaned out some lint errors. Can you review this code?

@andig andig self-assigned this Oct 2, 2024
@andig andig added the devices Specific device support label Oct 2, 2024
charger/peblar.go Outdated Show resolved Hide resolved
phases = 1
}

return int(phases), nil
Copy link
Member

Choose a reason for hiding this comment

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

This should return the switched phases- 1 or 3. If config is fixed to 1p connected switching will never be invoked. Why do we need this complexity here?

Copy link
Author

@PieVo PieVo Oct 2, 2024

Choose a reason for hiding this comment

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

Clear..
2 relays, 3phase connected: Both GetPhases & phase1p3p supported.
1 or 2 relays, 1phase connected: No GetPhases, no phase1p3p.

But what about the option where we switch all phases with 1 relay? Clearly no phase1p3p, but also no GetPhases?

Copy link
Member

Choose a reason for hiding this comment

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

But what about the option where we switch all phases with 1 relay? Clearly no phase1p3p, but also no GetPhases?

Not quite sure what you mean. If box is connected 1p that's part of loadpoint config that we rely on. GetPhases will only every be called if Phases1p3p is actually available.

var _ api.PhaseGetter = (*Peblar)(nil)

// GetPhases implements the api.PhaseGetter interface
func (wb *Peblar) GetPhases() (int, error) {
Copy link
Member

Choose a reason for hiding this comment

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

GetPhases should be decorated together with phases1p3p.

Copy link
Author

Choose a reason for hiding this comment

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

Ok.. I was under the assumption that GetPhases is always useful for EVCC to known how many phases are attached and usable. Let's say a 3-phase box which is connected with just 1 phase: it does not need GetPhases?

Copy link
Member

Choose a reason for hiding this comment

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

See above- not used.

charger/peblar.go Outdated Show resolved Hide resolved
charger/peblar.go Outdated Show resolved Hide resolved
charger/peblar.go Outdated Show resolved Hide resolved
charger/peblar.go Outdated Show resolved Hide resolved
charger/peblar.go Outdated Show resolved Hide resolved
charger/peblar.go Outdated Show resolved Hide resolved
@PieVo
Copy link
Author

PieVo commented Oct 3, 2024

Hmm, something bad happens to the currents, I'm getting a lot of these:

"Garage: charger logic error: current mismatch (got 15.1A measured, expected 12.2A)"

Need to find out if it is related to the recent changes.

@andig
Copy link
Member

andig commented Oct 3, 2024

There is a 1A difference between set current and max phase current observed:

		// use measured phase currents as fallback if charger does not provide max current or does not currently relay from vehicle (TWC3)
		if !isCg || errors.Is(err, api.ErrNotAvailable) {
			// validate if current too high by more than 1A (https://github.com/evcc-io/evcc/issues/14731)
			if current := lp.GetMaxPhaseCurrent(); current > lp.chargeCurrent+1.0 {
				if shouldBeConsistent {
					lp.log.WARN.Printf("charger logic error: current mismatch (got %.3gA measured, expected %.3gA)", current, lp.chargeCurrent)
				}
				lp.chargeCurrent = current
				lp.bus.Publish(evChargeCurrent, lp.chargeCurrent)
			}
		}

@andig andig changed the title charger: add peblar support Add Peblar Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devices Specific device support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants