MINIFICPP-558: initial provisioning for CoAP #437

phrocker · 2018-11-14T14:40:09Z

Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically master)?
Is your initial contribution a single, squashed commit?

For code changes:

If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
If applicable, have you updated the LICENSE file?
If applicable, have you updated the NOTICE file?

For documentation related changes:

Have you ensured that format looks appropriate for the output in which it is rendered?

Note:

Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.

phrocker · 2018-11-14T14:40:43Z

Adding tests presently, but providing a wip view.

phrocker · 2018-11-14T14:42:24Z

extensions/coap/CMakeLists.txt

+
+  set(BASE_DIR "${CMAKE_CURRENT_BINARY_DIR}/extensions/coap")
+  if (APPLE)
+  set(BYPRODUCT "${BASE_DIR}/extensions/coap/thirdparty/libcoap-src/.libs/libcoap-2-gnutls.a")


this may need a little modification as artifacts can vary depending on what is used for building TLS

arpadboda · 2018-11-14T15:36:36Z

docker/test/integration/minifi/test/__init__.py

@@ -42,6 +42,8 @@ def __init__(self, output_validator):

        self.segfault = False

+        self.segfault = False


This is already there two lines above.

arpadboda · 2018-11-14T15:41:56Z

extensions/coap/controllerservice/CoapConnector.h

+  // initialization mutex.
+  std::mutex initialization_mutex_;
+
+  std::atomic<bool> initialized_;


Do we plan to have multiple instances of COAPConnectorSerivce?

arpadboda · 2018-11-14T15:43:24Z

extensions/coap/nanofi/coap_functions.c

+
+void no_acknowledgement(struct coap_context_t *ctx, coap_session_t *session, coap_pdu_t *sent, coap_nack_reason_t reason, const coap_tid_t id){
+  if (global_ptrs.received_error){
+          global_ptrs.received_error(receiver, ctx, -1);


Indentation

arpadboda · 2018-11-14T15:43:43Z

extensions/coap/nanofi/coap_functions.c

+}
+
+int coap_event(struct coap_context_t *ctx, coap_event_t event, struct coap_session_t *session){
+    if (event == COAP_EVENT_SESSION_FAILED && global_ptrs.received_error){


Please indent with 2 spaces

arpadboda · 2018-11-14T15:44:21Z

extensions/coap/nanofi/coap_functions.c

+  size_t data_len;
+  coap_opt_iterator_t opt_iter;
+  coap_opt_t * block_opt = coap_check_option(received, COAP_OPTION_BLOCK1, &opt_iter);
+  if (block_opt) {


Why not

if(!block_opt)

?

arpadboda · 2018-11-14T15:52:38Z

extensions/coap/protocols/CoapC2Protocol.cpp

+  return -1;
+}
+
+int8_t CoapProtocol::getString(const C2ContentResponse * const cmp, const std::string &name, std::string *value) {


As it's C++, why value is a pointer (ref would be fine)?
Moreover I don't see why to have 0 or -1 return codes, empty string would be fine as well to indicate the lack of existence.

arpadboda · 2018-11-14T15:58:32Z

extensions/coap/protocols/CoapC2Protocol.cpp

+}
+
+const C2Payload * const CoapProtocol::getPayload(const std::string &name, const C2Payload &payload) {
+  if (payload.getLabel() == name) {


return getPayLoad(&payload);

As the pointer version exists, why to duplicate?

arpadboda · 2018-11-15T17:36:37Z

extensions/coap/controllerservice/CoapConnector.cpp

+
+  coap_str_const_t pld;
+  pld.length = size;
+  pld.s = payload;


Given .s is const uint8_t* ( https://github.com/obgm/libcoap/blob/develop/include/coap2/str.h ), could you change the signature of the function to handle the payload as const?

I'll have to look what it is now, but we'll probably keep with that the outer facing types are and translate as needed/ I don't know if we can move to const as we'll probably be taking ownership of that buffer in the next commit. that is what the initial version did per libcoap did ( not sure if they do that in coap 2 ) . It ultimately depends on what we end up with ( coap vs coap2 ), but appreciate the request!

arpadboda · 2018-11-15T17:40:38Z

extensions/coap/controllerservice/CoapConnector.cpp

+
+  coap_address_t dst_addr, src_addr;
+  coap_uri_t uri;
+  uri.host.s = reinterpret_cast<unsigned char *>(const_cast<char*>(host_.c_str()));


Uri.host is coap_str_const_t, which has "s" as const uint8_t*, so there is no need of const cast here, just reinterpret cast to "const uint8_t*".

Ah thanks for the diligence. This was changed last minute ( from coap to coap2 ) and I may return back to coap1 ( in libcoap) where the signature is unsigned char. I posted this early for someone to take a gander from an integration perspective, so this is a WIP.

test framework still under dev MINIFICPP-683: Remove extraneous std::move

phrocker commented Nov 14, 2018

View reviewed changes

arpadboda reviewed Nov 14, 2018

View reviewed changes

arpadboda reviewed Nov 15, 2018

View reviewed changes

phrocker force-pushed the MINIFICPP-558 branch 2 times, most recently from ff26f81 to 4a667b7 Compare November 20, 2018 03:31

MINIFICPP-558: initial provisioning for CoAP still a wip

d6d1419

test framework still under dev MINIFICPP-683: Remove extraneous std::move

phrocker force-pushed the MINIFICPP-558 branch 2 times, most recently from 3d07f09 to 77d6c08 Compare November 21, 2018 16:17

MINIFICPP-558: Add some test capabilities

13157cb

phrocker force-pushed the MINIFICPP-558 branch from 77d6c08 to 13157cb Compare November 21, 2018 16:20

phrocker closed this Nov 21, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MINIFICPP-558: initial provisioning for CoAP #437

MINIFICPP-558: initial provisioning for CoAP #437

phrocker commented Nov 14, 2018

phrocker commented Nov 14, 2018

phrocker Nov 14, 2018

arpadboda Nov 14, 2018

arpadboda Nov 14, 2018

arpadboda Nov 14, 2018

arpadboda Nov 14, 2018

arpadboda Nov 14, 2018

arpadboda Nov 14, 2018

arpadboda Nov 14, 2018

arpadboda Nov 15, 2018

phrocker Nov 15, 2018 •

edited

arpadboda Nov 15, 2018 •

edited

phrocker Nov 15, 2018 •

edited

		@@ -42,6 +42,8 @@ def __init__(self, output_validator):

		self.segfault = False

		self.segfault = False

MINIFICPP-558: initial provisioning for CoAP #437

MINIFICPP-558: initial provisioning for CoAP #437

Conversation

phrocker commented Nov 14, 2018

For all changes:

For code changes:

For documentation related changes:

Note:

phrocker commented Nov 14, 2018

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

phrocker Nov 15, 2018 • edited

Choose a reason for hiding this comment

arpadboda Nov 15, 2018 • edited

Choose a reason for hiding this comment

phrocker Nov 15, 2018 • edited

Choose a reason for hiding this comment

phrocker Nov 15, 2018 •

edited

arpadboda Nov 15, 2018 •

edited

phrocker Nov 15, 2018 •

edited