robosoft-ai / SMACC2

An Event-Driven, Asynchronous, Behavioral State Machine Library for ROS2 (Robotic Operating System) applications written in C++
https://smacc.dev
Apache License 2.0
223 stars 36 forks source link

Http client #522

Closed yassiezar closed 2 months ago

yassiezar commented 1 year ago

┆Issue is synchronized with this Jira Task by Unito

brettpac commented 1 year ago

Hey @yassiezar. Is there any issue with moving the client folder https://github.com/yassiezar/SMACC2/tree/http-client/smacc2_sm_reference_library/sm_atomic_http/include/sm_atomic_http/clients/client_behaviors to the https://github.com/robosoft-ai/SMACC2/tree/humble/smacc2_client_library folder?

yassiezar commented 1 year ago

No issues, but I'll probably need to modify it to make it a bit more generic so that its useful to other users

brettpac commented 1 year ago

Are there any complications with...

Moving the client https://github.com/yassiezar/SMACC2/blob/http-client/smacc2/include/smacc2/client_bases/smacc_http_client.hpp, to it's own folder in the smacc_client_library folder?
Moving the code to a cpp file?

Let's chat offline about this.
yassiezar commented 1 year ago

@brettpac I moved the packages and made separate CPP files. I still want to modify the client to be more generically useful

yassiezar commented 1 year ago

@brettpac just to update you, the example isn't working as expected anymore and HTTP responses aren't being received. I'm busy investigating, but I suspect it has to do with changes to Boost::beast's APIs. I'll ping you when its resolved

brettpac commented 2 months ago

I think it's just being held up by precommit...

Command: pre-commit run -a

brettpac commented 2 months ago

Hi @yassiezar , thank you for this pull request. Looks awesome. One last request...

For the cb_http_request found here... smacc2_sm_reference_library/sm_atomic_http/include/sm_atomic_http/clients/client_behaviors

Let's move that into a new folder entitled "client_behaviors" and place that folder inside of this one... smacc2_client_library/http_client/include/http_client/

So the end result would look like... smacc2_client_library/http_client/include/http_client/client_behaviors/cb_http_request.hpp

Thank you,

brettpac commented 2 months ago

Hi Jaycee, looks like we're missing some copyrights... smacc2_client_library/http_client/src/http_client/http_session.cpp: could not find copyright notices...

1 errors, checked 306 files
smacc2_client_library/http_client/src/http_client/ssl_http_session.cpp: could not find copyright notice
1 errors, checked 296 files
smacc2_client_library/http_client/include/http_client/ssl_http_session.hpp: could not find copyright notice
1 errors, checked 274 files
smacc2_client_library/http_client/include/http_client/http_session.hpp: could not find copyright notice
smacc2_client_library/http_client/include/http_client/http_session_base.hpp: could not find copyright notice
brettpac commented 2 months ago

// Copyright 2021 RobosoftAI Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // // http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License.

/***** *

Author: Jacobus Lock

**/

brettpac commented 2 months ago

Hmmm. As Im reading it, the failed binary check is tied to the sm_simple_action_client state machine package I just merged a few minutes ago...

yassiezar commented 2 months ago

@brettpac I just merged in your changes. That might fix the error

brettpac commented 2 months ago

I think to get this passing we just have to run precommit twice. Once to make fixes (which I can see it does from the logs), and the other to approve.

yassiezar commented 2 months ago

All the checks pass when i run it locally, not sure how to get it to pass here. I'm not sure why the clang-format check is failing above, does the command need to be edited to use clang-format instead of clang-format-12? Ubuntu 22.04 uses clang-format-14