You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by phrocker <gi...@git.apache.org> on 2018/11/21 16:27:45 UTC

[GitHub] nifi-minifi-cpp pull request #447: MINIFICPP-558: initial provisioning for C...

GitHub user phrocker opened a pull request:

    https://github.com/apache/nifi-minifi-cpp/pull/447

    MINIFICPP-558: initial provisioning for CoAP still a wip

    test framework still under dev
    
    MINIFICPP-683: Remove extraneous std::move
    
    MINIFICPP-558: Add some test capabilities
    
    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](http://www.apache.org/legal/resolved.html#category-a)?
    - [ ] 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.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/phrocker/nifi-minifi-cpp MINIFICPP-558-683

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/nifi-minifi-cpp/pull/447.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #447
    
----
commit 6eaca2afe64c42273fdd6ff111518ca3e0ca0a3d
Author: Marc Parisi <ph...@...>
Date:   2018-10-23T15:51:19Z

    MINIFICPP-558: initial provisioning for CoAP still a wip
    test framework still under dev
    
    MINIFICPP-683: Remove extraneous std::move
    
    MINIFICPP-558: Add some test capabilities

----


---

[GitHub] nifi-minifi-cpp pull request #447: MINIFICPP-558: initial provisioning for C...

Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on a diff in the pull request:

    https://github.com/apache/nifi-minifi-cpp/pull/447#discussion_r235480882
  
    --- Diff: extensions/coap/COAPLoader.h ---
    @@ -0,0 +1,81 @@
    +/**
    + *
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You 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.
    + */
    +#ifndef EXTENSIONS_COAPLOADER_H_
    +#define EXTENSIONS_COAPLOADER_H_
    +
    +#ifdef WIN32
    +#pragma comment(lib, "wldap32.lib" )
    +#pragma comment(lib, "crypt32.lib" )
    +#pragma comment(lib, "Ws2_32.lib")
    +#endif
    +
    +#include "core/ClassLoader.h"
    +#include "utils/StringUtils.h"
    +#include "protocols/CoapC2Protocol.h"
    +
    +/**
    + * Object factory class loader for this extension.
    + * Can add extensions to the default class loader through REGISTER_RESOURCE,
    + * but we want to ensure this factory is used specifically for CoAP and not the default loader.
    + */
    +class COAPObjectFactory : public core::ObjectFactory {
    + public:
    +  COAPObjectFactory() {
    +
    +  }
    +
    +  /**
    +   * Gets the name of the object.
    +   * @return class name of processor
    +   */
    +  virtual std::string getName() override{
    +    return "COAPObjectFactory";
    +  }
    +
    +  virtual std::string getClassName() override{
    +    return "COAPObjectFactory";
    +  }
    +  /**
    +   * Gets the class name for the object
    +   * @return class name for the processor.
    +   */
    +  virtual std::vector<std::string> getClassNames() override{
    +    std::vector<std::string> class_names;
    +    class_names.push_back("CoapProtocol");
    --- End diff --
    
    May want to make CoapServer accessible. 


---

[GitHub] nifi-minifi-cpp pull request #447: MINIFICPP-558: initial provisioning for C...

Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on a diff in the pull request:

    https://github.com/apache/nifi-minifi-cpp/pull/447#discussion_r235487951
  
    --- Diff: extensions/coap/controllerservice/CoapConnector.cpp ---
    @@ -0,0 +1,97 @@
    +/**
    + *
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You 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.
    + */
    +
    +#include "CoapConnector.h"
    +
    +#include "core/logging/LoggerConfiguration.h"
    +#include "core/controller/ControllerService.h"
    +#include <string>
    +#include <memory>
    +#include <set>
    +#include "core/Property.h"
    +#include "CoapConnector.h"
    +#include "io/validation.h"
    +#include "properties/Configure.h"
    +
    +namespace org {
    +namespace apache {
    +namespace nifi {
    +namespace minifi {
    +namespace coap {
    +namespace controllers {
    +
    +static core::Property RemoteServer;
    +static core::Property Port;
    +static core::Property MaxQueueSize;
    +
    +core::Property CoapConnectorService::RemoteServer(core::PropertyBuilder::createProperty("Remote Server")->withDescription("Remote CoAP server")->isRequired(true)->build());
    +core::Property CoapConnectorService::Port(core::PropertyBuilder::createProperty("Remote Port")->withDescription("Remote CoAP server port")->isRequired(true)->build());
    +core::Property CoapConnectorService::MaxQueueSize(core::PropertyBuilder::createProperty("Max Queue Size")->withDescription("Max queue size for received data ")->isRequired(true)->build());
    +
    +void CoapConnectorService::initialize() {
    +  if (initialized_)
    +    return;
    +
    +  CoapMessaging::getInstance();
    --- End diff --
    
    Mixed casing of CoAP and coap is quite confusing and hard to follow. 


---

[GitHub] nifi-minifi-cpp pull request #447: MINIFICPP-558: initial provisioning for C...

Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on a diff in the pull request:

    https://github.com/apache/nifi-minifi-cpp/pull/447#discussion_r235480740
  
    --- Diff: extensions/coap/CMakeLists.txt ---
    @@ -0,0 +1,93 @@
    +#
    +# Licensed to the Apache Software Foundation (ASF) under one
    +# or more contributor license agreements.  See the NOTICE file
    +# distributed with this work for additional information
    +# regarding copyright ownership.  The ASF licenses this file
    +# to you 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.
    +#
    +
    +
    +include(${CMAKE_SOURCE_DIR}/extensions/ExtensionHeader.txt)
    +include_directories(protocols nanofi controllerservice server)
    +include_directories(../http-curl/)
    +
    +file(GLOB CSOURCES "nanofi/*.c")
    +file(GLOB SOURCES "*.cpp" "protocols/*.cpp" "processors/*.cpp" "controllerservice/*.cpp" "server/*.cpp" )
    +
    +add_library(nanofi-coap-c STATIC ${CSOURCES})
    +add_library(minifi-coap STATIC ${SOURCES})
    +set_property(TARGET minifi-coap PROPERTY POSITION_INDEPENDENT_CODE ON)
    +
    +if(CMAKE_THREAD_LIBS_INIT)
    +  target_link_libraries(minifi-coap "${CMAKE_THREAD_LIBS_INIT}")
    +endif()
    +
    +  set(BASE_DIR "${CMAKE_CURRENT_BINARY_DIR}/extensions/coap")
    +  # determine version of GNUTLSs
    --- End diff --
    
    this is meant as a bit of a todo. 


---

[GitHub] nifi-minifi-cpp issue #447: MINIFICPP-558: initial provisioning for CoAP sti...

Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on the issue:

    https://github.com/apache/nifi-minifi-cpp/pull/447
  
    Since this is pretty large I'm doing a self review of this now. 


---