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 openDTU (for Hoymiles Micro Inverter) #2566

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from

Conversation

Sn0w3y
Copy link
Contributor

@Sn0w3y Sn0w3y commented Mar 8, 2024

This PR implements the openDTU (free DTU) dor Hoymiles Micro-Inverters).

The Module uses the API of the openDTU to communicate with the Inverters.

Thanks to @DerWahreKlinki for helping me with it :)

@sfeilmeier
Copy link
Contributor

@Sn0w3y: Thank you for the effort. Please read and execute the contribution guidelines carefully, especially this: https://openems.github.io/openems.io/openems/latest/contribute/coding-guidelines.html#_openems_edge_backend.
Also we cannot review and merge, before the Continuous Integration (CI) build is successful - i.e. there must be a green check icon next to "Build OpenEMS / build-java (pull_request)"

@Sn0w3y
Copy link
Contributor Author

Sn0w3y commented Mar 10, 2024

@Sn0w3y: Thank you for the effort. Please read and execute the contribution guidelines carefully, especially this: https://openems.github.io/openems.io/openems/latest/contribute/coding-guidelines.html#_openems_edge_backend. Also we cannot review and merge, before the Continuous Integration (CI) build is successful - i.e. there must be a green check icon next to "Build OpenEMS / build-java (pull_request)"

Thanks Stefan, is there a possibility to check the Build locally so i do not bother you by sending a PR with a Test which makes a red cross ? :D

Would be glad to know and i will fix it ASAP

@Sn0w3y Sn0w3y marked this pull request as draft March 11, 2024 01:34
@sfeilmeier
Copy link
Contributor

@Sn0w3y: In Eclipse you should always run Checkstyle and all JUnit tests (of your bundle). Afterwards run ./tools/prepare-commit.sh as suggested in Coding Guidelines. The script runs well in a WSL 2 environment (https://learn.microsoft.com/de-de/windows/wsl/install).

Changed 	public CompletableFuture<String> request(Endpoint endpoint) so it does not return null as otherwise test failes as excpected..
Copy link

Code Coverage

@Sn0w3y Sn0w3y marked this pull request as ready for review March 17, 2024 15:15
@Sn0w3y
Copy link
Contributor Author

Sn0w3y commented Mar 18, 2024

@sfeilmeier ready for Review i guess :)

Copy link

Code Coverage

Copy link
Contributor

@sebastianasen sebastianasen left a comment

Choose a reason for hiding this comment

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

Thanks for the implementation. 💪
Find my comments below.
Is there any limitation on specific hardware types?
Maybe you could also add further information like a link, supported versions or anything else that needs to be done before it can be used/configured.

}

switch (event.getTopic()) {
case EdgeEventConstants.TOPIC_CYCLE_AFTER_PROCESS_IMAGE:
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess that the case will never be reached, as it was not mentioned in the event topics in the class annotations

@EventTopics({ // EdgeEventConstants.TOPIC_CYCLE_BEFORE_PROCESS_IMAGE, // }) public class OpendtuImpl extends AbstractOpenemsComponent implements Opendtu, SinglePhaseMeter, ElectricityMeter, OpenemsComponent, EventHandler, TimedataProvider {

For now you could only react on case TOPIC_CYCLE_BEFORE_PROCESS_IMAGE mentioned in the annotation

*/
POWER_LIMIT(Doc.of(OpenemsType.INTEGER)//
.unit(Unit.PERCENT) //
.text("The Relative Limit of the max. Power.").accessMode(AccessMode.READ_WRITE)),
Copy link
Contributor

Choose a reason for hiding this comment

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

//

private volatile Timedata timedata = null;

private final CalculateEnergyFromPower calculateActualEnergy = new CalculateEnergyFromPower(this,
ElectricityMeter.ChannelId.ACTIVE_PRODUCTION_ENERGY);
Copy link
Contributor

Choose a reason for hiding this comment

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

Even if there is only Production, you could add CalculateEnergyFromPower with a Channel ACTIVE_CONSUMPTION_ENERGY, but it's not mandatory as it will never increase as long as the inverter is not used for heating the PV-moduls and consume power.

String serialNumber() default "";

@AttributeDefinition(name = "Meter-Type", description = "What is measured by this DTU?")
MeterType type() default MeterType.PRODUCTION;
Copy link
Contributor

Choose a reason for hiding this comment

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

If it is really only a pv inverter, like i would expect - you do not have to add the meter type in the config as it is always PRODUCTION.
Just set a fixed configuration property like this:
https://github.com/Sn0w3y/openems/blob/26cebce21d45f8f78edc93ed93e2084eb27c2750/io.openems.edge.pvinverter.fronius/src/io/openems/edge/pvinverter/fronius/PvInverterFroniusImpl.java#L44

Maybe you have to take care in the UI separately - i remember a method in the UI Part "isProduction".
Im not completely sure if there is still the need to do something in there.

@AttributeDefinition(name = "Meter-Type", description = "What is measured by this DTU?")
MeterType type() default MeterType.PRODUCTION;

@AttributeDefinition(name = "Initial Power Limit", description = "The initial power limit setting")
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the "power limit" used for?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants