[Sprint 22/23 / PD-405] [Feature] Add implementation GRPC #13

mbsaloka · 2024-04-27T07:35:03Z

Jira Link:

https://ichiro-its.atlassian.net/browse/PD-405?atlOrigin=eyJpIjoiZjcyMmNiZGY0YjQ5NDliYzkxYjhkZjBiODFmODk2OWUiLCJwIjoiaiJ9

Description

Added grpc to implement get, set and save functions. The gRPC server runs on port :5858 and consists of 3 RPCs, GetColorSetting, SetColorSetting, and SaveColorSetting. It has been tested using Postman and it works.

Example Message:

GetColorSetting

{}

SetColorSetting

{
    "max_hue": 123,
    "max_saturation": 123,
    "max_value": 123,
    "min_hue": 123,
    "min_saturation": 123,
    "min_value": 123,
    "name": "line"
}

SaveColorSetting

{
    "json_color": "{\"ball\":{\"max_hsv\":[179,100,100],\"min_hsv\":[151,40,10]},\"field\":{\"max_hsv\":[116,100,100],\"min_hsv\":[84,10,0]},\"line\":{\"max_hsv\":[260,60,100],\"min_hsv\":[180,0,55]}}"
}

Type of Change

Bugfix
Enhancement
New feature
Breaking change (fix or feature that would cause the existing functionality to not work as expected)

How Has This Been Tested?

New unit tests added.
Manual tested.

Checklist:

Using Branch Name Convention
- feature/JIRA-ID-SHORT-DESCRIPTION if has a JIRA ticket
- enhancement/SHORT-DESCRIPTION if has/has no JIRA ticket and contain enhancement
- hotfix/SHORT-DESCRIPTION if the change doesn't need to be tested (urgent)
I have commented on my code, particularly in hard-to-understand areas.
I have made the documentation for the corresponding changes.

FaaizHaikal

remove .vscode dir

include/ninshiki_cpp/config/grpc/config.hpp

FaaizHaikal · 2024-04-28T07:14:11Z

src/ninshiki_cpp/detector/color_detector.cpp

+      item.max_value = color.max_value;
+
+      std::cout << "[DEBUG] Color setting has been applied!" << std::endl;
+      std::cout << "name: " << item.name << std::endl;


remove these debugging lines

threeal

I suggest creating a dedicated gRPC utility package first before merging this change, as I see there are many duplications of CallData, CallDataBase, etc., across different projects that are going to implement gRPC (see ichiro-its/shisen_cpp#25 and ichiro-its/aruku#20).

borednuna · 2024-05-27T16:33:19Z

data/color_classifier.json

@@ -0,0 +1,38 @@
+{


We can remove this as we have moved the configurations to config repo

borednuna · 2024-05-27T16:35:42Z

include/ninshiki_cpp/utils/circles.hpp

@@ -0,0 +1,56 @@
+// Copyright (c) 2021 ICHIRO ITS


Isn't this done in branch refactor ninshiki ? Maybe remove all changes that are done in refactor ninshiki branch before merging this branch later

threeal · 2024-05-30T04:41:39Z

include/ninshiki_cpp/config/grpc/call_data_get_color_setting.hpp

+  void WaitForRequest();
+  void HandleRequest();


These lines should be marked as override.

Suggested change

void WaitForRequest();

void HandleRequest();

void WaitForRequest() override;

void HandleRequest() override;

threeal · 2024-05-30T04:41:55Z

include/ninshiki_cpp/config/grpc/call_data_save_color_setting.hpp

+  void WaitForRequest();
+  void HandleRequest();


threeal · 2024-05-30T04:42:07Z

include/ninshiki_cpp/config/grpc/call_data_set_color_setting.hpp

+  void WaitForRequest();
+  void HandleRequest();


threeal · 2024-05-30T04:42:55Z

include/ninshiki_cpp/config/grpc/config.hpp

+#include <absl/flags/flag.h>
+#include <absl/flags/parse.h>
+#include <absl/strings/str_format.h>
+#include <grpc/support/log.h>
+#include <grpcpp/grpcpp.h>
+#include <ninshiki_cpp/config/grpc/call_data.hpp>
+#include <ninshiki_cpp/config/grpc/call_data_base.hpp>
+#include <ninshiki_cpp/detector/color_detector.hpp>
+#include <ninshiki_interfaces/msg/color_setting.hpp>
+#include <ninshiki_interfaces/ninshiki.grpc.pb.h>
+#include <ninshiki_interfaces/ninshiki.pb.h>
+#include <nlohmann/json.hpp>
+#include <rclcpp/rclcpp.hpp>
+
+#include <chrono>
+#include <fstream>
+#include <future>
+#include <iostream>
+#include <map>
+#include <memory>
+#include <string>
+#include <thread>


Some of these headers may be unnecessary, don't put all of them in the header file.

threeal · 2024-05-30T04:44:45Z

src/ninshiki_cpp/config/grpc/call_data_base.cpp

+
+namespace ninshiki_cpp
+{
+CallDataBase::CallDataBase() {}


Just remove the constructor if it does not do anything.

after I deleted the constructor or deleted the call_data_base.cpp file, when building the package it produced an error

/usr/bin/ld: libninshiki_cpp.so: undefined reference to `ninshiki_cpp::CallDataBase::CallDataBase()'

You also need to remove the constructor from the class declaration in the .hpp file.

threeal · 2024-05-30T04:47:06Z

src/ninshiki_cpp/config/grpc/call_data_set_color_setting.cpp

+    std::string name = request_.name();
+    int min_hue = request_.min_hue();
+    int max_hue = request_.max_hue();
+    int min_saturation = request_.min_saturation();
+    int max_saturation = request_.max_saturation();
+    int min_value = request_.min_value();
+    int max_value = request_.max_value();
+
+    utils::Color color(
+      name,
+      min_hue,
+      max_hue,
+      min_saturation,
+      max_saturation,
+      min_value,
+      max_value
+    );


Simpler

Suggested change

std::string name = request_.name();

int min_hue = request_.min_hue();

int max_hue = request_.max_hue();

int min_saturation = request_.min_saturation();

int max_saturation = request_.max_saturation();

int min_value = request_.min_value();

int max_value = request_.max_value();

utils::Color color(

name,

min_hue,

max_hue,

min_saturation,

max_saturation,

min_value,

max_value

);

utils::Color color(

request_.name(),

request_.min_hue(),

request_.max_hue(),

request_.min_saturation(),

request_.max_saturation(),

request_.min_value(),

request_.max_value()

);

FaaizHaikal · 2024-06-05T12:24:29Z

I suggest creating a dedicated gRPC utility package first before merging this change, as I see there are many duplications of CallData, CallDataBase, etc., across different projects that are going to implement gRPC (see ichiro-its/shisen_cpp#25 and ichiro-its/aruku#20).

I think there are going to be a lot of changes while working on this. It might be worked on after this year's competition mas, which is around next month.

Need to update

mbsaloka added 7 commits April 22, 2024 22:11

feat: create grpc function

37052b8

feat: get and save function works

ae0d726

feat: set function works

35d7bf7

refactor: remove debug code and unnecassary code

29205a5

refactor: uncomment dnn detection and lbp detection codes

bcb3735

feat pulling the master branch and merge

73fa36e

docs: clean up code and add copyright

ee4172e

FaaizHaikal requested changes Apr 28, 2024

View reviewed changes

mbsaloka added 3 commits April 28, 2024 16:12

refactor: remove .vscode dir and debugging lines

c79bfd5

feat: add _exported to cmakelist

4dd50fe

Merge branch 'enhancement/refactor-ninshiki' into feature/using-grpc

d4daaca

threeal requested changes May 8, 2024

View reviewed changes

borednuna reviewed May 27, 2024

View reviewed changes

fix: remove unused file

6e89e69

threeal requested changes May 30, 2024

View reviewed changes

mbsaloka added 4 commits May 31, 2024 13:29

fix: add override

75f59d5

fix: remove unnecessary header and simplify codes

4f2885a

fix: remove call_data_base constrctor

78c8bae

fix: remove port parameter

c200bab

FaaizHaikal previously approved these changes Jun 4, 2024

View reviewed changes

FaaizHaikal force-pushed the feature/using-grpc branch from 343f44e to c200bab Compare June 6, 2024 03:30

mbsaloka closed this Jun 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Sprint 22/23 / PD-405] [Feature] Add implementation GRPC #13

[Sprint 22/23 / PD-405] [Feature] Add implementation GRPC #13

mbsaloka commented Apr 27, 2024

FaaizHaikal left a comment

FaaizHaikal Apr 28, 2024

threeal left a comment

borednuna May 27, 2024

borednuna May 27, 2024

threeal May 30, 2024

threeal May 30, 2024

threeal May 30, 2024

threeal May 30, 2024

threeal May 30, 2024

mbsaloka May 30, 2024

threeal May 31, 2024

threeal May 30, 2024

FaaizHaikal commented Jun 5, 2024

[Sprint 22/23 / PD-405] [Feature] Add implementation GRPC #13

[Sprint 22/23 / PD-405] [Feature] Add implementation GRPC #13

Conversation

mbsaloka commented Apr 27, 2024

Jira Link:

Description

Type of Change

How Has This Been Tested?

Checklist:

FaaizHaikal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

threeal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FaaizHaikal commented Jun 5, 2024