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

Implementation of grid CO2 traffic light component #2793

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

Conversation

a-l-s-o1
Copy link

This components implements a "traffic light" of the current CO2 emissions fo energy taken from the grid. The CO2 emission data is taken from corrently-api. The traffic light states "Green", "Yellow" and "Red" as well as the CO2 emissions are written to channels of the actual hour.

@Sn0w3y
Copy link
Contributor

Sn0w3y commented Sep 23, 2024

Hi,

thanks for your Contribution but could you maybe please follow the Coding Guidelines - especially running the "prepare-commit.sh" ? :)
This would remove unnecessary Files and also show you formatting issues.

Also in my Opinion we already have it here:

#2792

Greetings!

Copy link
Contributor

@Sn0w3y Sn0w3y left a comment

Choose a reason for hiding this comment

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

This is not a final review - i guess some other People have a little bit more to say about it :)

import java.time.temporal.ChronoUnit;
import java.io.BufferedReader;
import java.io.InputStreamReader;
import java.net.HttpURLConnection;
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a "HttpBridge" for this i guess

Copy link
Author

Choose a reason for hiding this comment

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

Hi Sn0wy, thanks for your review and your comments!
As I understood it, the HttpBridge is executed every cycle. Since the data is only updated every hour, the https request is currently only executed every hour. Did I get it wrong?

Copy link
Contributor

Choose a reason for hiding this comment

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

True, you still could implement a Counter for the requests :)

Copy link
Author

Choose a reason for hiding this comment

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

Sorry I guess i can't follow. Could you please explain it a bit more? So you mean using the httpBridge.subscribeJsonEveryCycle(); function and adding a counter its passed BiConsumer?

return new GridEmissionInformation(dateTime, consAdvice, co2);
}

private void updateDataBuffer() {
Copy link
Contributor

Choose a reason for hiding this comment

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

The current error handling logs exceptions but still continues execution. For example, if updateDataBuffer() encounters an error, it falls back to using stale data. While this prevents crashes, implementing a retry mechanism or a fallback to a cached version could improve reliability.

Copy link
Author

Choose a reason for hiding this comment

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

Good point. Currently it uses data from the buffer in case of error and retries in the next hour. If that is not good enough I will add functionality to retry multiple times.

Copy link
Contributor

Choose a reason for hiding this comment

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

So if it fails 4 Times in 4 hours one an hour it uses 4 hour old data? :)

Copy link
Author

Choose a reason for hiding this comment

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

That would be right. But I changed it now so that it retries every 5 minutes if an error occures. Otherwise it waits for


private final Logger log = LoggerFactory.getLogger(GreenConsumptionAdvisorImpl.class);

private static final String URL_API_RECOMMONDATION = "https://api.corrently.io/v2.0/gsi/advisor?zip=";
Copy link
Contributor

Choose a reason for hiding this comment

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

The method name writeRecommondationToChannel contains a typo ("Recommondation" should be "Recommendation")

Copy link
Author

Choose a reason for hiding this comment

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

Typo corrected

super.activate(context, config.id(), config.alias(), config.enabled());

// TODO validate zip code
this.urlString = URL_API_RECOMMONDATION + config.zip_code();
Copy link
Contributor

Choose a reason for hiding this comment

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

The method name writeRecommondationToChannel contains a typo ("Recommondation" should be "Recommendation")

Copy link
Author

Choose a reason for hiding this comment

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

Typo corrected

this.timeNextActualization = LocalDateTime.now().truncatedTo(ChronoUnit.HOURS).plusHours(1);
}
this.evaluateDataBuffer();
this.writeRecommondationToChannel();
Copy link
Contributor

Choose a reason for hiding this comment

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

The method name writeRecommondationToChannel contains a typo ("Recommondation" should be "Recommendation")

@a-l-s-o1
Copy link
Author

Hi,

thanks for your Contribution but could you maybe please follow the Coding Guidelines - especially running the "prepare-commit.sh" ? :) This would remove unnecessary Files and also show you formatting issues.

Also in my Opinion we already have it here:

#2792

Greetings!

Hi Sn0wy,

thanks for the review!

This code also was coded during the Hackathon. The similar merge reqeust contains excample code for an extended Getting started documentation but it does not contain the data buffering logic.

Regards!

Copy link
Contributor

@Sn0w3y Sn0w3y left a comment

Choose a reason for hiding this comment

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

Maybe make it incremental? :) Like if it fails for some reason increment the Count as in the Tibber Impl i guess it is aswell.

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

Successfully merging this pull request may close these issues.

2 participants