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

V1.6 in Pending state: Handling of the TriggerMessage.req to set connection state to accepted #421

Conversation

bWF0dGhpYXMK
Copy link
Contributor

If the answer of backend is bootnotificaiton.conf pending, then a TriggerMessage.req shall set the charge point to booted.

@bWF0dGhpYXMK
Copy link
Contributor Author

bWF0dGhpYXMK commented Jan 30, 2024

In order to accept TriggerMessage.req to send out the relevant requested *.req e.g. Heartbeat requires a bit of refactor as there can happen a interference with calls comming from outside. Stacked variable would be preferred to manage acceptance to allow sending of the package, although, it is in pending state. But this requires to change the calls eg. heartbeat(bool xAccept) and send(ocpp::Call call, bool xAccept), the same with status_notification and send_meter_value.

Comment on lines +996 to +998
if (enhanced_message.messageType == MessageType::GetConfiguration ||
enhanced_message.messageType == MessageType::ChangeConfiguration ||
enhanced_message.messageType == MessageType::TriggerMessage) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is too restrictive, the 1.6 spec in section 4.2 states

While in pending state, the following Central System initiated messages are not allowed:
RemoteStartTransaction.req and RemoteStopTransaction.req

There are no other restriction (other than not issuing chargepoint initiated requests)

Copy link
Contributor

Choose a reason for hiding this comment

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

@bWF0dGhpYXMK what's your opinion on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, a review of the latest code, there is a PR #520 managing that case with an answer on Rejected. From the ocpp1.6, for me it is not clear what means not allowed, initially I thought on "no" answer, which is two edged. Because an on the RPC framework point of view you require at least an answer such as Call Error.
So on that level, i think this PR can be closed. Either reject or CALL Error.

It remains the clarification on ReserverNow.req, Reset.req, ... what behavior here is the most likely in the sense of the describtion.

From this " The Central System MAY send request messages to retrieve information from the
Charge Point or change its configuration. The Charge Point SHOULD respond to these messages.". It sound like behave as in Remote(Start/Stop).req...

lib/ocpp/v16/charge_point_impl.cpp Outdated Show resolved Hide resolved
@bWF0dGhpYXMK bWF0dGhpYXMK changed the title Handling of the TriggerMessage.req to set connection state to accepted V1.6 in Pending state: Handling of the TriggerMessage.req to set connection state to accepted Feb 21, 2024
if (this->connection_state = ChargePointConnectionState::Pending) {
EVLOG_debug << "Set connection_state from pending to accepted";
this->connection_state = ChargePointConnectionState::Booted;
}

Choose a reason for hiding this comment

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

How do you come to this conclusion? From my understanding only the BootNotification.Conf should change this.

Copy link
Contributor Author

@bWF0dGhpYXMK bWF0dGhpYXMK Feb 22, 2024

Choose a reason for hiding this comment

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

Thank you for review. Initially, I interpretated that sentence: "The Charge
Point SHALL NOT send request messages to the Central System unless it has been instructed by the Central
System to do so with a TriggerMessage.req request." To use any TriggerMessage.req as trigger to set the connected state. However, after discussion, the sentence more reflects, the *.req initiated by a TriggerMessage.req. In that sense, the code change is not needed anymore. As proposed by you.

@hikinggrass hikinggrass self-assigned this Mar 25, 2024
@bWF0dGhpYXMK
Copy link
Contributor Author

Following discussions on Issue 478 related to the topic, the PR is set as done/closed.

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.

4 participants