Skip to content

Conversation

lbbrhzn
Copy link
Owner

@lbbrhzn lbbrhzn commented Feb 3, 2022

No description provided.

@lbbrhzn lbbrhzn requested a review from drc38 February 3, 2022 22:59
@codecov
Copy link

codecov bot commented Feb 3, 2022

Codecov Report

Merging #365 (be2df07) into main (fc27bca) will increase coverage by 0.32%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #365      +/-   ##
==========================================
+ Coverage   86.19%   86.51%   +0.32%     
==========================================
  Files          10       10              
  Lines        1188     1194       +6     
  Branches      174      177       +3     
==========================================
+ Hits         1024     1033       +9     
+ Misses         94       92       -2     
+ Partials       70       69       -1     
Impacted Files Coverage Δ
custom_components/ocpp/api.py 79.91% <75.00%> (+0.59%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc27bca...be2df07. Read the comment docs.

return False
"""Trigger status notifications for all connectors."""
return_value = True
for id in range(0, 1):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use self._metrics[cdet.connectors.value].value here instead?

Copy link
Owner Author

Choose a reason for hiding this comment

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

It does need to be known, so the number of connectors should be retrieved before sending the triggers.

@drc38
Copy link
Collaborator

drc38 commented Feb 4, 2022

Perhaps in the @on(Action.StatusNotification) use the extra attributes of the connector status metric to handle statuses and errors for more than 1 connector?

@drc38
Copy link
Collaborator

drc38 commented Feb 4, 2022

Not sure what you mean. The functionality to store status and error code separately for main (id=0) and connector (id>0) is already in.

Is it for only 1 (ie 0 and 1) connector or will it handle say 0, 1 and 2?

@lbbrhzn
Copy link
Owner Author

lbbrhzn commented Feb 4, 2022

Yes. We could support more than one connector, but to display the info in separate sensors we would need to know how many there at setup, or create the additional sensors on the fly, which is messy.

So, let's just keep the two sensors for connector=0 and connector=1 as is, and store additional info for other connectors in extra_attributes.

@drc38
Copy link
Collaborator

drc38 commented Feb 4, 2022

So, let's just keep the two sensors for connector=0 and connector=1 as is, and store additional info for other connectors in extra_attributes.

Agree 👍

@lbbrhzn lbbrhzn merged commit 76ee686 into main Feb 5, 2022
@lbbrhzn lbbrhzn deleted the trigger-status-notification-for-multiple-connectors branch February 5, 2022 17:52
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