You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by GitBox <gi...@apache.org> on 2021/01/03 20:39:32 UTC

[GitHub] [celix] pnoltes opened a new pull request #310: Adds initial impl for the C++ headers only wrappers

pnoltes opened a new pull request #310:
URL: https://github.com/apache/celix/pull/310


   ### WIP
   This PR is still work in progress.
   
   ### TODO
   
   - [ ] Update documentation for C++ wrapper usage
   - [ ] Update C++ examples (dynamic services & locking)
   - [ ] Add support for service factories
   - [ ] Add a Celix exception and use this.
   
   ### Intro 
   ThisPR introduces a (header only) C++ wrapper for the C Celix framework.
   
   A while back the idea was to introduce a C++ framework of Celix. 
   This idea has been abandoned (a new C++20 framework should be a different project).
   
   To make life easier when using C++ with Celix this PR introduces C++ wrappers around most of the C Celix concepts:
    - Bundle Context
    - Bundle
    - Framework
    - Properties
    - Filter
    - Service Registration
    - Service Tracker
    - Bundle Tracker
    - Meta Tracker (service tracker tracker)
    - BundleActivator
   
   ### Async
   The C++ wrapper works on top of the async C celix API; this means that creating service registration and trackers can be done inside a lock. 
   This also means that the C++ object wrapping service registration/trackers have a state that will change (e.g. REGISTERING->REGISTERED->UNREGISTERING->UNREGISTERED).
   
   Examples:
   ```C++
       celix::Properties props{};
       props["key1"] = "value1";
       auto svc = std::make_shared<CInterface>(CInterface{nullptr, nullptr});
       auto svcReg1 = ctx->registerService<CInterface>(svc)
               .setProperties(props)
               .addProperty("key2", "value2")
               .build();
   ```
   ```C++
       std::atomic<int> count{0};
       auto tracker3 = ctx->trackServices<CInterface>()
               .addAddCallback([&count](const std::shared_ptr<CInterface>&) {
                   count += 1;
               })
               .addRemCallback([&count](const std::shared_ptr<CInterface>&) {
                   count += 1;
               })
               .build();
   ```
   
   
   ### Fluent Builder API
   Most call in the C++ BundleContext use a fluent builder API to make configuring complex service registration, trackers, etc more concise and readable. 
   
   ### BundleActivator Changes
   The CELIX_GEN_CXX_BUNDLE_ACTIVATOR macro has change so that it still support the C++ DepedencyManager activator, but also support bundle activator which take a `std::shared_ptr<celix::BundleContext>` as argument.
   
   
   
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] Oipo commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
Oipo commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r580069489



##########
File path: libs/framework/include/celix_bundle_activator.h
##########
@@ -139,75 +139,6 @@ celix_status_t celix_bundleActivator_destroy(void *userData, celix_bundle_contex
 
 #ifdef __cplusplus
 }
-
-
-/**
- * This macro generates the required bundle activator functions for C++.
- * This can be used to more type safe bundle activator entries.
- *
- * The macro will create the following bundle activator functions:
- * - bundleActivator_create which allocates a pointer to the provided type.
- * - bundleActivator_start/stop which will call the respectively provided typed start/stop functions.
- * - bundleActivator_destroy will free the allocated for the provided type.
- *
- * @param type The activator type (e.g. 'ShellActivator'). A type which should have a constructor with a single arugment of std::shared_ptr<DependencyManager>.
- */
-#define CELIX_GEN_CXX_BUNDLE_ACTIVATOR(actType)                                                                        \

Review comment:
       Oh, I must've missed the moving of the define. I thought it was deleted entirely.

##########
File path: libs/framework/include/celix_bundle_activator.h
##########
@@ -139,75 +139,6 @@ celix_status_t celix_bundleActivator_destroy(void *userData, celix_bundle_contex
 
 #ifdef __cplusplus
 }
-
-
-/**
- * This macro generates the required bundle activator functions for C++.
- * This can be used to more type safe bundle activator entries.
- *
- * The macro will create the following bundle activator functions:
- * - bundleActivator_create which allocates a pointer to the provided type.
- * - bundleActivator_start/stop which will call the respectively provided typed start/stop functions.
- * - bundleActivator_destroy will free the allocated for the provided type.
- *
- * @param type The activator type (e.g. 'ShellActivator'). A type which should have a constructor with a single arugment of std::shared_ptr<DependencyManager>.
- */
-#define CELIX_GEN_CXX_BUNDLE_ACTIVATOR(actType)                                                                        \

Review comment:
       Oh, I must've missed the moving of the define/macro. I thought it was deleted entirely.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] pnoltes commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r579830235



##########
File path: libs/framework/doxygen.md
##########
@@ -0,0 +1,66 @@
+<!--
+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.
+-->
+
+# Intro
+
+The main way to use Celix is through the bundle context of a bundle.
+When a bundle is started the bundle context will be injected in the bundle activator.
+
+Apache Celix is a C framework with a C and C++ (C++11) API. 

Review comment:
       The reason why i used C++11 is because the C++ dependency manager stuff if C++11.
   But yeah, this can be C++17 IMO.
   
   Maybe this can be updated in a new PR, to prevent this PR from growing larger than it already is. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] pnoltes commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r579829884



##########
File path: examples/celix-examples/services_example_cxx/src/DynamicConsumerBundleActivator.cc
##########
@@ -0,0 +1,159 @@
+/*
+ * 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 <utility>
+
+#include "celix/IShellCommand.h"
+#include "celix/BundleActivator.h"
+#include "examples/ICalc.h"
+
+namespace /*anon*/ {
+
+    class DynamicConsumer {
+    public:
+        explicit DynamicConsumer(int _id) : id{_id} {}
+
+        ~DynamicConsumer() {
+            stop();
+        }
+
+        void start() {
+            std::lock_guard<std::mutex> lck{mutex};
+            calcThread = std::thread{&DynamicConsumer::run, this};

Review comment:
       Thanks. Good catch. Updated the usage of thread/start/stop in the examples. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] codecov-io edited a comment on pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #310:
URL: https://github.com/apache/celix/pull/310#issuecomment-753673666


   # [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=h1) Report
   > Merging [#310](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=desc) (31b4afe) into [master](https://codecov.io/gh/apache/celix/commit/8c5871439db9bf7308a762dce71be9806a8663e8?el=desc) (8c58714) will **increase** coverage by `0.75%`.
   > The diff coverage is `92.88%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/celix/pull/310/graphs/tree.svg?width=650&height=150&src=pr&token=JdsiThga8P)](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #310      +/-   ##
   ==========================================
   + Coverage   69.17%   69.93%   +0.75%     
   ==========================================
     Files         151      168      +17     
     Lines       30697    31652     +955     
   ==========================================
   + Hits        21235    22136     +901     
   - Misses       9462     9516      +54     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ubsub/pubsub\_spi/src/pubsub\_interceptors\_handler.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2ludGVyY2VwdG9yc19oYW5kbGVyLmM=) | `48.93% <0.00%> (ø)` | |
   | [...ubsub/pubsub\_utils/src/pubsub\_serializer\_handler.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfc2VyaWFsaXplcl9oYW5kbGVyLmM=) | `90.95% <0.00%> (ø)` | |
   | [bundles/shell/shell/src/c\_shell.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9zaGVsbC9zaGVsbC9zcmMvY19zaGVsbC5j) | `85.79% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/Component.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Db21wb25lbnQuaA==) | `100.00% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/Component\_Impl.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Db21wb25lbnRfSW1wbC5o) | `90.90% <ø> (ø)` | |
   | [...ramework/include/celix/dm/DependencyManager\_Impl.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9EZXBlbmRlbmN5TWFuYWdlcl9JbXBsLmg=) | `100.00% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/ProvidedService.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2UuaA==) | `100.00% <ø> (ø)` | |
   | [.../framework/include/celix/dm/ProvidedService\_Impl.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2VfSW1wbC5o) | `100.00% <ø> (ø)` | |
   | [...ibs/framework/include/celix/dm/ServiceDependency.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9TZXJ2aWNlRGVwZW5kZW5jeS5o) | `84.21% <ø> (ø)` | |
   | [libs/framework/include/celix\_bundle\_context.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeF9idW5kbGVfY29udGV4dC5o) | `100.00% <ø> (ø)` | |
   | ... and [53 more](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=footer). Last update [8c58714...31b4afe](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] codecov-io edited a comment on pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #310:
URL: https://github.com/apache/celix/pull/310#issuecomment-753673666


   # [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=h1) Report
   > Merging [#310](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=desc) (dd71537) into [master](https://codecov.io/gh/apache/celix/commit/3bec2e0e1445a81dfe076ae725412323882cb74f?el=desc) (3bec2e0) will **increase** coverage by `0.73%`.
   > The diff coverage is `92.77%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/celix/pull/310/graphs/tree.svg?width=650&height=150&src=pr&token=JdsiThga8P)](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #310      +/-   ##
   ==========================================
   + Coverage   70.14%   70.87%   +0.73%     
   ==========================================
     Files         154      172      +18     
     Lines       30449    31446     +997     
   ==========================================
   + Hits        21357    22288     +931     
   - Misses       9092     9158      +66     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ubsub/pubsub\_spi/src/pubsub\_interceptors\_handler.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2ludGVyY2VwdG9yc19oYW5kbGVyLmM=) | `48.93% <0.00%> (ø)` | |
   | [...ubsub/pubsub\_utils/src/pubsub\_serializer\_handler.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfc2VyaWFsaXplcl9oYW5kbGVyLmM=) | `90.95% <0.00%> (ø)` | |
   | [bundles/shell/shell/src/c\_shell.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9zaGVsbC9zaGVsbC9zcmMvY19zaGVsbC5j) | `85.79% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/Component.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Db21wb25lbnQuaA==) | `72.72% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/ProvidedService.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2UuaA==) | `100.00% <ø> (ø)` | |
   | [.../framework/include/celix/dm/ProvidedService\_Impl.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2VfSW1wbC5o) | `97.05% <ø> (-0.17%)` | :arrow_down: |
   | [...ibs/framework/include/celix/dm/ServiceDependency.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9TZXJ2aWNlRGVwZW5kZW5jeS5o) | `92.30% <ø> (ø)` | |
   | [libs/framework/include/celix\_bundle\_context.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeF9idW5kbGVfY29udGV4dC5o) | `100.00% <ø> (ø)` | |
   | [libs/framework/src/celix\_log.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2xvZy5j) | `89.39% <ø> (ø)` | |
   | [libs/framework/src/dm\_component\_impl.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2RtX2NvbXBvbmVudF9pbXBsLmM=) | `81.08% <ø> (+0.24%)` | :arrow_up: |
   | ... and [52 more](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=footer). Last update [3bec2e0...5127cdc](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] codecov-io edited a comment on pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #310:
URL: https://github.com/apache/celix/pull/310#issuecomment-753673666


   # [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=h1) Report
   > Merging [#310](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=desc) (dca3993) into [master](https://codecov.io/gh/apache/celix/commit/3bec2e0e1445a81dfe076ae725412323882cb74f?el=desc) (3bec2e0) will **increase** coverage by `0.76%`.
   > The diff coverage is `93.67%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/celix/pull/310/graphs/tree.svg?width=650&height=150&src=pr&token=JdsiThga8P)](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #310      +/-   ##
   ==========================================
   + Coverage   70.14%   70.90%   +0.76%     
   ==========================================
     Files         154      172      +18     
     Lines       30449    31442     +993     
   ==========================================
   + Hits        21357    22295     +938     
   - Misses       9092     9147      +55     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ubsub/pubsub\_spi/src/pubsub\_interceptors\_handler.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2ludGVyY2VwdG9yc19oYW5kbGVyLmM=) | `48.93% <0.00%> (ø)` | |
   | [...ubsub/pubsub\_utils/src/pubsub\_serializer\_handler.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfc2VyaWFsaXplcl9oYW5kbGVyLmM=) | `90.95% <0.00%> (ø)` | |
   | [bundles/shell/shell/src/c\_shell.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9zaGVsbC9zaGVsbC9zcmMvY19zaGVsbC5j) | `85.79% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/Component.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Db21wb25lbnQuaA==) | `72.72% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/ProvidedService.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2UuaA==) | `100.00% <ø> (ø)` | |
   | [.../framework/include/celix/dm/ProvidedService\_Impl.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2VfSW1wbC5o) | `97.05% <ø> (-0.17%)` | :arrow_down: |
   | [...ibs/framework/include/celix/dm/ServiceDependency.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9TZXJ2aWNlRGVwZW5kZW5jeS5o) | `92.30% <ø> (ø)` | |
   | [libs/framework/include/celix\_bundle\_context.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeF9idW5kbGVfY29udGV4dC5o) | `100.00% <ø> (ø)` | |
   | [libs/framework/src/celix\_log.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2xvZy5j) | `89.39% <ø> (ø)` | |
   | [libs/framework/src/dm\_component\_impl.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2RtX2NvbXBvbmVudF9pbXBsLmM=) | `81.08% <ø> (+0.24%)` | :arrow_up: |
   | ... and [51 more](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=footer). Last update [3bec2e0...dca3993](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] codecov-io edited a comment on pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #310:
URL: https://github.com/apache/celix/pull/310#issuecomment-753673666


   # [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=h1) Report
   > Merging [#310](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=desc) (0fbde7b) into [master](https://codecov.io/gh/apache/celix/commit/0870cd960acae2f9ba74e444e22c7d4f32a1de0a?el=desc) (0870cd9) will **increase** coverage by `0.66%`.
   > The diff coverage is `92.64%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/celix/pull/310/graphs/tree.svg?width=650&height=150&src=pr&token=JdsiThga8P)](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #310      +/-   ##
   ==========================================
   + Coverage   68.69%   69.35%   +0.66%     
   ==========================================
     Files         149      162      +13     
     Lines       30639    31367     +728     
   ==========================================
   + Hits        21046    21756     +710     
   - Misses       9593     9611      +18     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ubsub/pubsub\_spi/src/pubsub\_interceptors\_handler.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2ludGVyY2VwdG9yc19oYW5kbGVyLmM=) | `48.93% <0.00%> (ø)` | |
   | [...ubsub/pubsub\_utils/src/pubsub\_serializer\_handler.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfc2VyaWFsaXplcl9oYW5kbGVyLmM=) | `90.95% <0.00%> (ø)` | |
   | [bundles/shell/shell/src/activator.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9zaGVsbC9zaGVsbC9zcmMvYWN0aXZhdG9yLmM=) | `92.15% <ø> (-0.08%)` | :arrow_down: |
   | [libs/framework/include/celix/dm/Component.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Db21wb25lbnQuaA==) | `100.00% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/Component\_Impl.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Db21wb25lbnRfSW1wbC5o) | `90.90% <ø> (ø)` | |
   | [...ramework/include/celix/dm/DependencyManager\_Impl.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9EZXBlbmRlbmN5TWFuYWdlcl9JbXBsLmg=) | `100.00% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/ProvidedService.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2UuaA==) | `100.00% <ø> (ø)` | |
   | [.../framework/include/celix/dm/ProvidedService\_Impl.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2VfSW1wbC5o) | `100.00% <ø> (ø)` | |
   | [...ibs/framework/include/celix/dm/ServiceDependency.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9TZXJ2aWNlRGVwZW5kZW5jeS5o) | `84.21% <ø> (ø)` | |
   | [libs/framework/include/celix\_bundle\_context.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeF9idW5kbGVfY29udGV4dC5o) | `100.00% <ø> (ø)` | |
   | ... and [44 more](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=footer). Last update [0870cd9...0fbde7b](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] pnoltes commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r579856601



##########
File path: libs/framework/include/celix/Framework.h
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+#include <memory>
+
+#include "celix_framework.h"
+
+namespace celix {
+
+    class BundleContext; //forward declaration
+
+    /**
+     * @brief A Celix framework instance. A framework is also known as a system bundle.
+     *
+     * Celix framework instances are created using a FrameworkFactory.
+     *
+     * @note Thread safe.
+     */
+    class Framework {
+    public:
+        Framework(std::shared_ptr<celix::BundleContext> _fwCtx, celix_framework_t* _cFw) :
+            fwCtx{std::move(_fwCtx)},
+            cFw{std::shared_ptr<celix_framework_t >{_cFw, [](celix_framework_t*){/*nop*/}}}
+            {}
+
+        /**
+         * @brief Get the framework UUID.
+         */
+        std::string getUUID() const {
+            return std::string{celix_framework_getUUID(cFw.get())};
+        }
+
+        /**
+         * @brief Get the bundle context for the framework.
+         */
+        std::shared_ptr<celix::BundleContext> getFrameworkBundleContext() const {
+            return fwCtx;
+        }
+
+        /**
+         * @brief Fire a generic Celix framework event.
+         *
+         * The event will be added to the event loop and handled on the event loop thread.
+         *
+         * if bndId >=0 the bundle usage count will be increased while the event is not yet processed or finished processing.
+         * The eventName is expected to be const char* valid during til the event is finished processing.
+         *
+         * if eventId >=0 this will be used, otherwise a new event id will be generated.
+         *
+         * @return the event id (can be used in Framework::waitForEvent).
+         */
+        long fireGenericEvent(long bndId, const char* eventName, std::function<void()> processEventCallback, long eventId = -1) {
+            auto* callbackOnHeap = new std::function<void()>{};
+            *callbackOnHeap = std::move(processEventCallback);
+            return celix_framework_fireGenericEvent(
+                    cFw.get(),
+                    eventId,
+                    bndId,
+                    eventName,
+                    static_cast<void*>(callbackOnHeap),
+                    [](void *data) {
+                        auto* callback = static_cast<std::function<void()>*>(data);
+                        (*callback)();
+                        delete callback;
+                    },
+                    nullptr,
+                    nullptr);
+        }
+
+        /**
+         * @brief Block until the framework is stopped.
+         */
+         void waitForStop() {
+            celix_framework_waitForStop(cFw.get());
+        }
+
+        /**
+         * @brief Wait until all Celix event for this framework are completed.
+         */
+        void waitForEvent(long eventId) {
+            celix_framework_waitForGenericEvent(cFw.get(), eventId);
+        }
+
+        /**
+         * @brief Get the C framework.
+         *
+         * @warning Try not the depend on the C API from a C++ bundle. If features are missing these should be added to
+         * the C++ API.

Review comment:
       I would like to add these, but the PR is already quite large so I would prefer to do this in a separate PR. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] pnoltes commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r579855867



##########
File path: libs/framework/include/celix/TrackerBuilders.h
##########
@@ -0,0 +1,358 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+#include <string>
+#include <vector>
+#include <memory>
+
+#include "Trackers.h"
+
+namespace celix {
+
+    /**
+     * @brief Fluent builder API to track services.
+     *
+     * @see celix::BundleContext::trackServices for more info.
+     * @tparam I The service type to track.
+     * @note Not thread safe.
+     */
+    template<typename I>
+    class ServiceTrackerBuilder {
+    private:
+        friend class BundleContext;
+
+        //NOTE private to prevent move so that a build() call cannot be forgotten
+        ServiceTrackerBuilder(ServiceTrackerBuilder&&) = default;
+    public:
+        explicit ServiceTrackerBuilder(std::shared_ptr<celix_bundle_context_t> _cCtx, std::string _name) :
+                cCtx{std::move(_cCtx)},
+                name{std::move(_name)} {}
+
+        ServiceTrackerBuilder& operator=(ServiceTrackerBuilder&&) = delete;
+        ServiceTrackerBuilder(const ServiceTrackerBuilder&) = delete;
+        ServiceTrackerBuilder operator=(const ServiceTrackerBuilder&) = delete;
+
+        /**
+         * @brief Set filter to be used to matching services.
+         *
+         * The filter must be LDAP filter.
+         * Example:
+         *      "(property_key=value)"
+         */
+        ServiceTrackerBuilder& setFilter(std::string f) { filter = celix::Filter{std::move(f)}; return *this; }
+
+        /**
+         * @brief Set filter to be used to matching services.
+         */
+        ServiceTrackerBuilder& setFilter(celix::Filter f) { filter = std::move(f); return *this; }
+
+        /**
+         * @brief Adds a add callback function, which will be called - on the Celix event thread -
+         * when a new service match is found.
+         *
+         * The add callback function has 1 argument: A shared ptr to the added service.
+         */
+        ServiceTrackerBuilder& addAddCallback(std::function<void(std::shared_ptr<I>)> add) {
+            addCallbacks.emplace_back([add](std::shared_ptr<I> svc, std::shared_ptr<const celix::Properties>, std::shared_ptr<const celix::Bundle>) {
+                add(svc);
+            });
+            return *this;
+        }
+
+        /**
+         * @brief Adds a add callback function, which will be called - on the Celix event thread -
+         * when a new service match is found.
+         *
+         * The add callback function has 2 arguments:
+         *  - A shared ptr to the added service.
+         *  - A shared ptr to the added service properties.
+         */
+        ServiceTrackerBuilder& addAddWithPropertiesCallback(std::function<void(std::shared_ptr<I>, std::shared_ptr<const celix::Properties>)> add) {
+            addCallbacks.emplace_back([add](std::shared_ptr<I> svc, std::shared_ptr<const celix::Properties> props, std::shared_ptr<const celix::Bundle>) {
+                add(svc, props);
+            });
+            return *this;
+        }
+
+        /**
+         * @brief Adds a add callback function, which will be called - on the Celix event thread -
+         * when a new service match is found.
+         *
+         * The add callback function has 3 arguments:
+         *  - A shared ptr to the added service.
+         *  - A shared ptr to the added service properties.
+         *  - A shared ptr to the bundle owning the added service.
+         */
+        ServiceTrackerBuilder& addAddWithOwnerCallback(std::function<void(std::shared_ptr<I>, std::shared_ptr<const celix::Properties>, std::shared_ptr<const celix::Bundle>)> add) {
+            addCallbacks.emplace_back(std::move(add));
+            return *this;
+        }
+
+        /**
+         * @brief Adds a remove callback function, which will be called - on the Celix event thread -
+         * when a service match is being removed.
+         *
+         * The remove callback function has 1 arguments: A shared ptr to the removing service.
+         */
+        ServiceTrackerBuilder& addRemCallback(std::function<void(std::shared_ptr<I>)> remove) {
+            remCallbacks.emplace_back([remove](std::shared_ptr<I> svc, std::shared_ptr<const celix::Properties>, std::shared_ptr<const celix::Bundle>) {
+                remove(svc);
+            });
+            return *this;
+        }
+
+        /**
+         * @brief Adds a remove callback function, which will be called - on the Celix event thread -
+         * when a service match is being removed.
+         *
+         * The remove callback function has 2 arguments:
+         *  - A shared ptr to the removing service.
+         *  - A shared ptr to the removing service properties.
+         */
+        ServiceTrackerBuilder& addRemWithPropertiesCallback(std::function<void(std::shared_ptr<I>, std::shared_ptr<const celix::Properties>)> remove) {
+            remCallbacks.emplace_back([remove](std::shared_ptr<I> svc, std::shared_ptr<const celix::Properties> props, std::shared_ptr<const celix::Bundle>) {
+                remove(svc, props);
+            });
+            return *this;
+        }
+
+        /**
+         * @brief Adds a remove callback function, which will be called - on the Celix event thread -
+         * when a service match is being removed.
+         *
+         * The remove callback function has 3 arguments:
+         *  - A shared ptr to the removing service.
+         *  - A shared ptr to the removing service properties.
+         *  - A shared ptr to the bundle owning the removing service.
+         */
+        ServiceTrackerBuilder& addRemWithOwnerCallback(std::function<void(std::shared_ptr<I>, std::shared_ptr<const celix::Properties>, std::shared_ptr<const celix::Bundle>)> remove) {
+            remCallbacks.emplace_back(std::move(remove));
+            return *this;
+        }
+
+        /**
+         * @brief Adds a set callback function, which will be called - on the Celix event thread -
+         * when there is a new highest ranking service match.
+         * This can can also be an empty match (nullptr).
+         *
+         * The set callback function has 2 arguments: A shared ptr to the highest

Review comment:
       updated. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] codecov-io commented on pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #310:
URL: https://github.com/apache/celix/pull/310#issuecomment-753673666


   # [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=h1) Report
   > Merging [#310](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=desc) (0b0e894) into [master](https://codecov.io/gh/apache/celix/commit/0870cd960acae2f9ba74e444e22c7d4f32a1de0a?el=desc) (0870cd9) will **increase** coverage by `0.66%`.
   > The diff coverage is `97.86%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/celix/pull/310/graphs/tree.svg?width=650&height=150&src=pr&token=JdsiThga8P)](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #310      +/-   ##
   ==========================================
   + Coverage   68.69%   69.35%   +0.66%     
   ==========================================
     Files         149      159      +10     
     Lines       30639    31219     +580     
   ==========================================
   + Hits        21046    21652     +606     
   + Misses       9593     9567      -26     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [bundles/shell/shell/src/activator.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9zaGVsbC9zaGVsbC9zcmMvYWN0aXZhdG9yLmM=) | `92.15% <ø> (-0.08%)` | :arrow_down: |
   | [libs/framework/include/celix/dm/Component.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Db21wb25lbnQuaA==) | `100.00% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/Component\_Impl.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Db21wb25lbnRfSW1wbC5o) | `90.90% <ø> (ø)` | |
   | [...ramework/include/celix/dm/DependencyManager\_Impl.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9EZXBlbmRlbmN5TWFuYWdlcl9JbXBsLmg=) | `100.00% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/ProvidedService.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2UuaA==) | `100.00% <ø> (ø)` | |
   | [.../framework/include/celix/dm/ProvidedService\_Impl.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2VfSW1wbC5o) | `100.00% <ø> (ø)` | |
   | [...ibs/framework/include/celix/dm/ServiceDependency.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9TZXJ2aWNlRGVwZW5kZW5jeS5o) | `84.21% <ø> (ø)` | |
   | [libs/framework/include/celix\_bundle\_context.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeF9idW5kbGVfY29udGV4dC5o) | `100.00% <ø> (ø)` | |
   | [libs/framework/src/dm\_component\_impl.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2RtX2NvbXBvbmVudF9pbXBsLmM=) | `57.03% <ø> (-0.02%)` | :arrow_down: |
   | [libs/framework/src/dm\_service\_dependency.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2RtX3NlcnZpY2VfZGVwZW5kZW5jeS5j) | `51.61% <ø> (-0.40%)` | :arrow_down: |
   | ... and [33 more](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=footer). Last update [0870cd9...0b0e894](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] codecov-io edited a comment on pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #310:
URL: https://github.com/apache/celix/pull/310#issuecomment-753673666


   # [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=h1) Report
   > Merging [#310](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=desc) (f8c9945) into [master](https://codecov.io/gh/apache/celix/commit/3bec2e0e1445a81dfe076ae725412323882cb74f?el=desc) (3bec2e0) will **increase** coverage by `0.77%`.
   > The diff coverage is `93.67%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/celix/pull/310/graphs/tree.svg?width=650&height=150&src=pr&token=JdsiThga8P)](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #310      +/-   ##
   ==========================================
   + Coverage   70.14%   70.91%   +0.77%     
   ==========================================
     Files         154      172      +18     
     Lines       30449    31442     +993     
   ==========================================
   + Hits        21357    22296     +939     
   - Misses       9092     9146      +54     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ubsub/pubsub\_spi/src/pubsub\_interceptors\_handler.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2ludGVyY2VwdG9yc19oYW5kbGVyLmM=) | `48.93% <0.00%> (ø)` | |
   | [...ubsub/pubsub\_utils/src/pubsub\_serializer\_handler.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfc2VyaWFsaXplcl9oYW5kbGVyLmM=) | `90.95% <0.00%> (ø)` | |
   | [bundles/shell/shell/src/c\_shell.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9zaGVsbC9zaGVsbC9zcmMvY19zaGVsbC5j) | `85.79% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/Component.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Db21wb25lbnQuaA==) | `72.72% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/ProvidedService.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2UuaA==) | `100.00% <ø> (ø)` | |
   | [.../framework/include/celix/dm/ProvidedService\_Impl.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2VfSW1wbC5o) | `97.05% <ø> (-0.17%)` | :arrow_down: |
   | [...ibs/framework/include/celix/dm/ServiceDependency.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9TZXJ2aWNlRGVwZW5kZW5jeS5o) | `92.30% <ø> (ø)` | |
   | [libs/framework/include/celix\_bundle\_context.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeF9idW5kbGVfY29udGV4dC5o) | `100.00% <ø> (ø)` | |
   | [libs/framework/src/celix\_log.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2xvZy5j) | `89.39% <ø> (ø)` | |
   | [libs/framework/src/dm\_component\_impl.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2RtX2NvbXBvbmVudF9pbXBsLmM=) | `81.08% <ø> (+0.24%)` | :arrow_up: |
   | ... and [52 more](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=footer). Last update [3bec2e0...9f7349e](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] codecov-io edited a comment on pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #310:
URL: https://github.com/apache/celix/pull/310#issuecomment-753673666


   # [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=h1) Report
   > Merging [#310](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=desc) (19e092c) into [master](https://codecov.io/gh/apache/celix/commit/3bec2e0e1445a81dfe076ae725412323882cb74f?el=desc) (3bec2e0) will **increase** coverage by `0.73%`.
   > The diff coverage is `92.85%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/celix/pull/310/graphs/tree.svg?width=650&height=150&src=pr&token=JdsiThga8P)](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #310      +/-   ##
   ==========================================
   + Coverage   70.14%   70.87%   +0.73%     
   ==========================================
     Files         154      172      +18     
     Lines       30449    31445     +996     
   ==========================================
   + Hits        21357    22288     +931     
   - Misses       9092     9157      +65     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ubsub/pubsub\_spi/src/pubsub\_interceptors\_handler.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2ludGVyY2VwdG9yc19oYW5kbGVyLmM=) | `48.93% <0.00%> (ø)` | |
   | [...ubsub/pubsub\_utils/src/pubsub\_serializer\_handler.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfc2VyaWFsaXplcl9oYW5kbGVyLmM=) | `90.95% <0.00%> (ø)` | |
   | [bundles/shell/shell/src/c\_shell.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9zaGVsbC9zaGVsbC9zcmMvY19zaGVsbC5j) | `85.79% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/Component.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Db21wb25lbnQuaA==) | `72.72% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/ProvidedService.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2UuaA==) | `100.00% <ø> (ø)` | |
   | [.../framework/include/celix/dm/ProvidedService\_Impl.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2VfSW1wbC5o) | `97.05% <ø> (-0.17%)` | :arrow_down: |
   | [...ibs/framework/include/celix/dm/ServiceDependency.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9TZXJ2aWNlRGVwZW5kZW5jeS5o) | `92.30% <ø> (ø)` | |
   | [libs/framework/include/celix\_bundle\_context.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeF9idW5kbGVfY29udGV4dC5o) | `100.00% <ø> (ø)` | |
   | [libs/framework/src/celix\_log.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2xvZy5j) | `89.39% <ø> (ø)` | |
   | [libs/framework/src/dm\_component\_impl.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2RtX2NvbXBvbmVudF9pbXBsLmM=) | `81.08% <ø> (+0.24%)` | :arrow_up: |
   | ... and [52 more](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=footer). Last update [3bec2e0...19e092c](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] pnoltes commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r579829975



##########
File path: examples/celix-examples/services_example_cxx/src/SimpleConsumerBundleActivator.cc
##########
@@ -0,0 +1,99 @@
+/*
+ * 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 "celix/BundleActivator.h"
+#include "examples/ICalc.h"
+
+class SimpleConsumer {
+public:
+    ~SimpleConsumer() {
+        stop();
+    }
+
+    SimpleConsumer() = default;
+    SimpleConsumer(SimpleConsumer&&) = delete;
+    SimpleConsumer& operator=(SimpleConsumer&&) = delete;
+    SimpleConsumer(const SimpleConsumer&) = delete;
+    SimpleConsumer& operator=(const SimpleConsumer&) = delete;
+
+    void start() {
+        std::lock_guard<std::mutex> lck{mutex};
+        calcThread = std::thread{&SimpleConsumer::run, this};
+    }
+
+    void stop() {
+        active = false;
+        std::lock_guard<std::mutex> lck{mutex};

Review comment:
       Correct, I adjusted this. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] pnoltes commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r579830066



##########
File path: libs/framework/include/celix/Utils.h
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+
+#include <string>
+#include <string.h>
+#include <iostream>
+
+namespace celix {
+    /**
+     * @brief Returns the deferred type name for the template I
+     */
+    template<typename INTERFACE_TYPENAME>
+    std::string typeName() {
+        std::string result;
+
+#ifdef __GXX_RTTI

Review comment:
       Yes, this is a good improvement. updated




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] codecov-io edited a comment on pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #310:
URL: https://github.com/apache/celix/pull/310#issuecomment-753673666


   # [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=h1) Report
   > Merging [#310](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=desc) (0fbde7b) into [master](https://codecov.io/gh/apache/celix/commit/0870cd960acae2f9ba74e444e22c7d4f32a1de0a?el=desc) (0870cd9) will **increase** coverage by `0.66%`.
   > The diff coverage is `92.64%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/celix/pull/310/graphs/tree.svg?width=650&height=150&src=pr&token=JdsiThga8P)](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #310      +/-   ##
   ==========================================
   + Coverage   68.69%   69.35%   +0.66%     
   ==========================================
     Files         149      162      +13     
     Lines       30639    31367     +728     
   ==========================================
   + Hits        21046    21756     +710     
   - Misses       9593     9611      +18     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ubsub/pubsub\_spi/src/pubsub\_interceptors\_handler.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2ludGVyY2VwdG9yc19oYW5kbGVyLmM=) | `48.93% <0.00%> (ø)` | |
   | [...ubsub/pubsub\_utils/src/pubsub\_serializer\_handler.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfc2VyaWFsaXplcl9oYW5kbGVyLmM=) | `90.95% <0.00%> (ø)` | |
   | [bundles/shell/shell/src/activator.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9zaGVsbC9zaGVsbC9zcmMvYWN0aXZhdG9yLmM=) | `92.15% <ø> (-0.08%)` | :arrow_down: |
   | [libs/framework/include/celix/dm/Component.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Db21wb25lbnQuaA==) | `100.00% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/Component\_Impl.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Db21wb25lbnRfSW1wbC5o) | `90.90% <ø> (ø)` | |
   | [...ramework/include/celix/dm/DependencyManager\_Impl.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9EZXBlbmRlbmN5TWFuYWdlcl9JbXBsLmg=) | `100.00% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/ProvidedService.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2UuaA==) | `100.00% <ø> (ø)` | |
   | [.../framework/include/celix/dm/ProvidedService\_Impl.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2VfSW1wbC5o) | `100.00% <ø> (ø)` | |
   | [...ibs/framework/include/celix/dm/ServiceDependency.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9TZXJ2aWNlRGVwZW5kZW5jeS5o) | `84.21% <ø> (ø)` | |
   | [libs/framework/include/celix\_bundle\_context.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeF9idW5kbGVfY29udGV4dC5o) | `100.00% <ø> (ø)` | |
   | ... and [44 more](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=footer). Last update [0870cd9...0fbde7b](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] pnoltes commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r579855761



##########
File path: libs/framework/include/celix_bundle_activator.h
##########
@@ -139,75 +139,6 @@ celix_status_t celix_bundleActivator_destroy(void *userData, celix_bundle_contex
 
 #ifdef __cplusplus
 }
-
-
-/**
- * This macro generates the required bundle activator functions for C++.
- * This can be used to more type safe bundle activator entries.
- *
- * The macro will create the following bundle activator functions:
- * - bundleActivator_create which allocates a pointer to the provided type.
- * - bundleActivator_start/stop which will call the respectively provided typed start/stop functions.
- * - bundleActivator_destroy will free the allocated for the provided type.
- *
- * @param type The activator type (e.g. 'ShellActivator'). A type which should have a constructor with a single arugment of std::shared_ptr<DependencyManager>.
- */
-#define CELIX_GEN_CXX_BUNDLE_ACTIVATOR(actType)                                                                        \

Review comment:
       Both style should be possible, thanks to std::enable_if:
   https://github.com/apache/celix/blob/feature/cxx_headers/libs/framework/include/celix/BundleActivator.h#L52
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] pnoltes merged pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
pnoltes merged pull request #310:
URL: https://github.com/apache/celix/pull/310


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] pnoltes commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r579829664



##########
File path: examples/celix-examples/dm_example_cxx/phase1/src/Phase1Activator.cc
##########
@@ -38,17 +38,22 @@ struct InvalidCServ {
 };
 
 Phase1Activator::Phase1Activator(std::shared_ptr<celix::dm::DependencyManager> mng) {
-    auto cmp = std::shared_ptr<Phase1Cmp>(new Phase1Cmp());
+    dm = mng;
+    auto& cmp = mng->createComponent<Phase1Cmp>();  //using a pointer a instance. Also supported is lazy initialization (default constructor needed) or a rvalue reference (move)

Review comment:
       removed comment. this was outdated.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] Oipo commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
Oipo commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r579843727



##########
File path: libs/framework/doxygen.md
##########
@@ -0,0 +1,66 @@
+<!--
+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.
+-->
+
+# Intro
+
+The main way to use Celix is through the bundle context of a bundle.
+When a bundle is started the bundle context will be injected in the bundle activator.
+
+Apache Celix is a C framework with a C and C++ (C++11) API. 

Review comment:
       I agree, another PR would be more suitable.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] pnoltes commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r579857438



##########
File path: libs/framework/include/celix/Bundle.h
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+#include <memory>
+
+#include "celix_bundle.h"
+
+namespace celix {
+
+    /**
+     * @brief An installed bundle in the Celix framework.
+     *
+     * Each bundle installed in the Celix framework must have an associated Bundle object.
+     * A bundle must have a unique identity, a long, chosen by the Celix framework.
+     *
+     * @note Thread safe.
+     */
+    class Bundle {

Review comment:
       That is still TODO. And to prevent this PR from growing larger than it already is I would like to add these in a separate PR.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] pnoltes commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r579856104



##########
File path: libs/framework/include/celix/ServiceFactory.h
##########
@@ -0,0 +1,36 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+#include "celix/Bundle.h"
+#include "celix/Properties.h"
+
+namespace celix {
+
+    template<typename I>
+    class ServiceFactory {
+    public:
+        virtual ~ServiceFactory() = default;
+
+        virtual std::shared_ptr<I> createBundleSpecificService(const celix::Bundle& requestingBundle, const celix::Properties svcFactoryProperties) = 0;
+        //TODO,TBD need ungetService variant?

Review comment:
       I agree. Note that service factory is still a TODO, but I removed the TBD. 
   This should be possible with a custom deleter. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] Oipo commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
Oipo commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r576918222



##########
File path: bundles/shell/shell/src/Shell.cc
##########
@@ -0,0 +1,284 @@
+/*
+ * 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 <memory>
+#include <unordered_map>
+
+#include "celix/BundleActivator.h"
+#include "celix/IShellCommand.h"
+#include "std_commands.h"
+#include "celix_shell_command.h"
+#include "celix_log_service.h"
+#include "celix_shell_command.h"
+#include "celix_shell.h"
+
+namespace celix {
+
+    class Shell {
+    public:
+
+        /**
+         * Try to execute a command using the provided command line.
+         */
+        celix_status_t executeCommand(const char *commandLine, FILE *out, FILE *err) {
+            std::vector<std::string> tokens = tokenize(commandLine);
+            if (tokens.empty()) {
+                fprintf(err, "Invalid commandLine '%s'\n", commandLine);
+                return CELIX_ILLEGAL_ARGUMENT;
+            }
+
+            Entry entry = findCommand(tokens[0], err);
+            if (entry.cCommand) {
+                 bool successful = entry.cCommand->executeCommand(entry.cCommand->handle, commandLine, out, err);
+                 return successful ? CELIX_SUCCESS : CELIX_SERVICE_EXCEPTION;
+            } else if (entry.cxxCommand) {
+                std::vector<std::string> arguments{};
+                auto start = ++tokens.begin();
+                arguments.insert(arguments.begin(), start, tokens.end());
+                entry.cxxCommand->executeCommand(commandLine, arguments, out, err);

Review comment:
       `arguments` is copied for each invocation. Either use `std::move` or make the second argument a reference type in `IShellCommand`

##########
File path: examples/celix-examples/services_example_cxx/src/SimpleConsumerBundleActivator.cc
##########
@@ -0,0 +1,99 @@
+/*
+ * 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 "celix/BundleActivator.h"
+#include "examples/ICalc.h"
+
+class SimpleConsumer {
+public:
+    ~SimpleConsumer() {
+        stop();
+    }
+
+    SimpleConsumer() = default;
+    SimpleConsumer(SimpleConsumer&&) = delete;
+    SimpleConsumer& operator=(SimpleConsumer&&) = delete;
+    SimpleConsumer(const SimpleConsumer&) = delete;
+    SimpleConsumer& operator=(const SimpleConsumer&) = delete;
+
+    void start() {
+        std::lock_guard<std::mutex> lck{mutex};
+        calcThread = std::thread{&SimpleConsumer::run, this};
+    }
+
+    void stop() {
+        active = false;
+        std::lock_guard<std::mutex> lck{mutex};

Review comment:
       Possible deadlock:
   1. calcThread checks active
   2. stop sets active to false
   3. stop locks mutex and waits for calcThread
   4. calcThread tries to lock mutex
   
   Probably bettter to use the atomic exchange thing to determine if calcThread needs to be joined.

##########
File path: libs/framework/include/celix/Framework.h
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+#include <memory>
+
+#include "celix_framework.h"
+
+namespace celix {
+
+    class BundleContext; //forward declaration
+
+    /**
+     * @brief A Celix framework instance. A framework is also known as a system bundle.
+     *
+     * Celix framework instances are created using a FrameworkFactory.
+     *
+     * @note Thread safe.
+     */
+    class Framework {
+    public:
+        Framework(std::shared_ptr<celix::BundleContext> _fwCtx, celix_framework_t* _cFw) :
+            fwCtx{std::move(_fwCtx)},
+            cFw{std::shared_ptr<celix_framework_t >{_cFw, [](celix_framework_t*){/*nop*/}}}
+            {}
+
+        /**
+         * @brief Get the framework UUID.
+         */
+        std::string getUUID() const {
+            return std::string{celix_framework_getUUID(cFw.get())};
+        }
+
+        /**
+         * @brief Get the bundle context for the framework.
+         */
+        std::shared_ptr<celix::BundleContext> getFrameworkBundleContext() const {
+            return fwCtx;
+        }
+
+        /**
+         * @brief Fire a generic Celix framework event.
+         *
+         * The event will be added to the event loop and handled on the event loop thread.
+         *
+         * if bndId >=0 the bundle usage count will be increased while the event is not yet processed or finished processing.
+         * The eventName is expected to be const char* valid during til the event is finished processing.
+         *
+         * if eventId >=0 this will be used, otherwise a new event id will be generated.
+         *
+         * @return the event id (can be used in Framework::waitForEvent).
+         */
+        long fireGenericEvent(long bndId, const char* eventName, std::function<void()> processEventCallback, long eventId = -1) {
+            auto* callbackOnHeap = new std::function<void()>{};
+            *callbackOnHeap = std::move(processEventCallback);
+            return celix_framework_fireGenericEvent(
+                    cFw.get(),
+                    eventId,
+                    bndId,
+                    eventName,
+                    static_cast<void*>(callbackOnHeap),
+                    [](void *data) {
+                        auto* callback = static_cast<std::function<void()>*>(data);
+                        (*callback)();
+                        delete callback;
+                    },
+                    nullptr,
+                    nullptr);
+        }
+
+        /**
+         * @brief Block until the framework is stopped.
+         */
+         void waitForStop() {
+            celix_framework_waitForStop(cFw.get());
+        }
+
+        /**
+         * @brief Wait until all Celix event for this framework are completed.
+         */
+        void waitForEvent(long eventId) {
+            celix_framework_waitForGenericEvent(cFw.get(), eventId);
+        }
+
+        /**
+         * @brief Get the C framework.
+         *
+         * @warning Try not the depend on the C API from a C++ bundle. If features are missing these should be added to
+         * the C++ API.

Review comment:
       Plenty of functions available in the C API are missing (such as `celix_framework_useBundle`). Is it not wiser to already add these to the C++ API?

##########
File path: libs/framework/include/celix/ServiceFactory.h
##########
@@ -0,0 +1,36 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+#include "celix/Bundle.h"
+#include "celix/Properties.h"
+
+namespace celix {
+
+    template<typename I>
+    class ServiceFactory {
+    public:
+        virtual ~ServiceFactory() = default;
+
+        virtual std::shared_ptr<I> createBundleSpecificService(const celix::Bundle& requestingBundle, const celix::Properties svcFactoryProperties) = 0;
+        //TODO,TBD need ungetService variant?

Review comment:
       If possible to put this in the deleter of the shared_ptr, I would prefer that.

##########
File path: libs/framework/doxygen.md
##########
@@ -0,0 +1,66 @@
+<!--
+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.
+-->
+
+# Intro
+
+The main way to use Celix is through the bundle context of a bundle.
+When a bundle is started the bundle context will be injected in the bundle activator.
+
+Apache Celix is a C framework with a C and C++ (C++11) API. 

Review comment:
       Is it intentional to keep this at c++11 when some parts are already C++17?

##########
File path: libs/framework/include/celix/Bundle.h
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+#include <memory>
+
+#include "celix_bundle.h"
+
+namespace celix {
+
+    /**
+     * @brief An installed bundle in the Celix framework.
+     *
+     * Each bundle installed in the Celix framework must have an associated Bundle object.
+     * A bundle must have a unique identity, a long, chosen by the Celix framework.
+     *
+     * @note Thread safe.
+     */
+    class Bundle {

Review comment:
       What is the intended way for a user to get:
   * bundle state
   * bundle group
   * list of registered services in bundle
   * list of registered trackers in bundle

##########
File path: examples/celix-examples/services_example_cxx/src/DynamicConsumerBundleActivator.cc
##########
@@ -0,0 +1,159 @@
+/*
+ * 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 <utility>
+
+#include "celix/IShellCommand.h"
+#include "celix/BundleActivator.h"
+#include "examples/ICalc.h"
+
+namespace /*anon*/ {
+
+    class DynamicConsumer {
+    public:
+        explicit DynamicConsumer(int _id) : id{_id} {}
+
+        ~DynamicConsumer() {
+            stop();
+        }
+
+        void start() {
+            std::lock_guard<std::mutex> lck{mutex};
+            calcThread = std::thread{&DynamicConsumer::run, this};

Review comment:
       lock_guard implies that calcThread needs to be thread-safe. But `stop()` isn't using a mutex. Perhaps use the `exchange` method on the `active` variable to determine what to do?

##########
File path: examples/celix-examples/dm_example_cxx/phase1/src/Phase1Activator.cc
##########
@@ -38,17 +38,22 @@ struct InvalidCServ {
 };
 
 Phase1Activator::Phase1Activator(std::shared_ptr<celix::dm::DependencyManager> mng) {
-    auto cmp = std::shared_ptr<Phase1Cmp>(new Phase1Cmp());
+    dm = mng;
+    auto& cmp = mng->createComponent<Phase1Cmp>();  //using a pointer a instance. Also supported is lazy initialization (default constructor needed) or a rvalue reference (move)

Review comment:
       Not only is the first sentence hard to follow, a reference is actually used instead of a pointer.

##########
File path: libs/framework/include/celix/TrackerBuilders.h
##########
@@ -0,0 +1,358 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+#include <string>
+#include <vector>
+#include <memory>
+
+#include "Trackers.h"
+
+namespace celix {
+
+    /**
+     * @brief Fluent builder API to track services.
+     *
+     * @see celix::BundleContext::trackServices for more info.
+     * @tparam I The service type to track.
+     * @note Not thread safe.
+     */
+    template<typename I>
+    class ServiceTrackerBuilder {
+    private:
+        friend class BundleContext;
+
+        //NOTE private to prevent move so that a build() call cannot be forgotten
+        ServiceTrackerBuilder(ServiceTrackerBuilder&&) = default;
+    public:
+        explicit ServiceTrackerBuilder(std::shared_ptr<celix_bundle_context_t> _cCtx, std::string _name) :
+                cCtx{std::move(_cCtx)},
+                name{std::move(_name)} {}
+
+        ServiceTrackerBuilder& operator=(ServiceTrackerBuilder&&) = delete;
+        ServiceTrackerBuilder(const ServiceTrackerBuilder&) = delete;
+        ServiceTrackerBuilder operator=(const ServiceTrackerBuilder&) = delete;
+
+        /**
+         * @brief Set filter to be used to matching services.
+         *
+         * The filter must be LDAP filter.
+         * Example:
+         *      "(property_key=value)"
+         */
+        ServiceTrackerBuilder& setFilter(std::string f) { filter = celix::Filter{std::move(f)}; return *this; }
+
+        /**
+         * @brief Set filter to be used to matching services.
+         */
+        ServiceTrackerBuilder& setFilter(celix::Filter f) { filter = std::move(f); return *this; }
+
+        /**
+         * @brief Adds a add callback function, which will be called - on the Celix event thread -
+         * when a new service match is found.
+         *
+         * The add callback function has 1 argument: A shared ptr to the added service.
+         */
+        ServiceTrackerBuilder& addAddCallback(std::function<void(std::shared_ptr<I>)> add) {
+            addCallbacks.emplace_back([add](std::shared_ptr<I> svc, std::shared_ptr<const celix::Properties>, std::shared_ptr<const celix::Bundle>) {
+                add(svc);
+            });
+            return *this;
+        }
+
+        /**
+         * @brief Adds a add callback function, which will be called - on the Celix event thread -
+         * when a new service match is found.
+         *
+         * The add callback function has 2 arguments:
+         *  - A shared ptr to the added service.
+         *  - A shared ptr to the added service properties.
+         */
+        ServiceTrackerBuilder& addAddWithPropertiesCallback(std::function<void(std::shared_ptr<I>, std::shared_ptr<const celix::Properties>)> add) {
+            addCallbacks.emplace_back([add](std::shared_ptr<I> svc, std::shared_ptr<const celix::Properties> props, std::shared_ptr<const celix::Bundle>) {
+                add(svc, props);
+            });
+            return *this;
+        }
+
+        /**
+         * @brief Adds a add callback function, which will be called - on the Celix event thread -
+         * when a new service match is found.
+         *
+         * The add callback function has 3 arguments:
+         *  - A shared ptr to the added service.
+         *  - A shared ptr to the added service properties.
+         *  - A shared ptr to the bundle owning the added service.
+         */
+        ServiceTrackerBuilder& addAddWithOwnerCallback(std::function<void(std::shared_ptr<I>, std::shared_ptr<const celix::Properties>, std::shared_ptr<const celix::Bundle>)> add) {
+            addCallbacks.emplace_back(std::move(add));
+            return *this;
+        }
+
+        /**
+         * @brief Adds a remove callback function, which will be called - on the Celix event thread -
+         * when a service match is being removed.
+         *
+         * The remove callback function has 1 arguments: A shared ptr to the removing service.
+         */
+        ServiceTrackerBuilder& addRemCallback(std::function<void(std::shared_ptr<I>)> remove) {
+            remCallbacks.emplace_back([remove](std::shared_ptr<I> svc, std::shared_ptr<const celix::Properties>, std::shared_ptr<const celix::Bundle>) {
+                remove(svc);
+            });
+            return *this;
+        }
+
+        /**
+         * @brief Adds a remove callback function, which will be called - on the Celix event thread -
+         * when a service match is being removed.
+         *
+         * The remove callback function has 2 arguments:
+         *  - A shared ptr to the removing service.
+         *  - A shared ptr to the removing service properties.
+         */
+        ServiceTrackerBuilder& addRemWithPropertiesCallback(std::function<void(std::shared_ptr<I>, std::shared_ptr<const celix::Properties>)> remove) {
+            remCallbacks.emplace_back([remove](std::shared_ptr<I> svc, std::shared_ptr<const celix::Properties> props, std::shared_ptr<const celix::Bundle>) {
+                remove(svc, props);
+            });
+            return *this;
+        }
+
+        /**
+         * @brief Adds a remove callback function, which will be called - on the Celix event thread -
+         * when a service match is being removed.
+         *
+         * The remove callback function has 3 arguments:
+         *  - A shared ptr to the removing service.
+         *  - A shared ptr to the removing service properties.
+         *  - A shared ptr to the bundle owning the removing service.
+         */
+        ServiceTrackerBuilder& addRemWithOwnerCallback(std::function<void(std::shared_ptr<I>, std::shared_ptr<const celix::Properties>, std::shared_ptr<const celix::Bundle>)> remove) {
+            remCallbacks.emplace_back(std::move(remove));
+            return *this;
+        }
+
+        /**
+         * @brief Adds a set callback function, which will be called - on the Celix event thread -
+         * when there is a new highest ranking service match.
+         * This can can also be an empty match (nullptr).
+         *
+         * The set callback function has 2 arguments: A shared ptr to the highest

Review comment:
       2 arguments? I only see 1

##########
File path: libs/framework/include/celix/Trackers.h
##########
@@ -0,0 +1,783 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+#include <memory>
+#include <mutex>
+#include <atomic>
+#include <cassert>
+#include <set>
+#include <unordered_map>
+
+#include "celix_utils.h"
+#include "celix/Properties.h"
+#include "celix/Utils.h"
+#include "celix/Bundle.h"
+#include "celix/Constants.h"
+#include "celix/Filter.h"
+#include "celix_bundle_context.h"
+
+namespace celix {
+
+
+    /**
+     * @brief The tracker state.
+     */
+    enum class TrackerState {
+        OPENING,
+        OPEN,
+        CLOSING,
+        CLOSED
+    };
+
+    /**
+     * @brief The AbstractTracker class is the base of all C++ Celix trackers.
+     *
+     * It defines how trackers are closed and manages the tracker state.
+     *
+     * This class can be used to create a vector of different (shared ptr)
+     * trackers (i.e. ServiceTracker, BundleTracker, MetaTracker).
+     *
+     * AbstractTracker <-----------------------------------
+     *  ^                               |                |
+     *  |                               |                |
+     *  GenericServiceTracker      BundleTracker    MetaTracker
+     *  ^
+     *  |
+     *  ServiceTracker<I>
+     *
+     * @note Thread safe.
+     */
+    class AbstractTracker {
+    public:
+        explicit AbstractTracker(std::shared_ptr<celix_bundle_context_t> _cCtx) :
+            cCtx{std::move(_cCtx)} {}
+
+        virtual ~AbstractTracker() noexcept = default;
+
+        /**
+         * @brief Check if the tracker is open (state == OPEN)
+         */
+        bool isOpen() const {
+            std::lock_guard<std::mutex> lck{mutex};
+            return state == TrackerState::OPEN;
+        }
+
+        /**
+         * @brief Get the current state of the tracker.
+         */
+        TrackerState getState() const {
+            std::lock_guard<std::mutex> lck{mutex};
+            return state;
+        }
+
+        /**
+         * @brief Close the tracker (of the state is not CLOSED or CLOSING).
+         *
+         * This will be done sync so then the close() method return the
+         * tracker is closed and all the needed callbacks have been called.
+         */
+        void close() {
+            long localTrkId = -1;
+            {
+                std::lock_guard<std::mutex> lck{mutex};
+                if (state == TrackerState::OPEN || state == TrackerState::OPENING) {
+                    //not yet closed
+                    state = TrackerState::CLOSING;
+                    localTrkId = trkId;
+                    trkId = -1;
+                }
+            }
+            if (localTrkId >= 0) {
+                celix_bundleContext_stopTracker(cCtx.get(), localTrkId);
+                {
+                    std::lock_guard<std::mutex> lck{mutex};
+                    state = TrackerState::CLOSED;
+                }
+            }
+        }
+
+        /**
+         * @brief Open the tracker (if the state is not OPEN or OPENING).
+         *
+         * This is done async, meaning that the actual opening of the tracker will be
+         * done a a Celix event processed on the Celix event thread.
+         *
+         * @throws celix::Exception
+         */
+        virtual void open() {}

Review comment:
       Any particular reason this is not a pure virtual function?

##########
File path: libs/framework/include/celix/Utils.h
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+
+#include <string>
+#include <string.h>
+#include <iostream>
+
+namespace celix {
+    /**
+     * @brief Returns the deferred type name for the template I
+     */
+    template<typename INTERFACE_TYPENAME>
+    std::string typeName() {
+        std::string result;
+
+#ifdef __GXX_RTTI

Review comment:
       I think this would work for clang as well, if one were to use something like https://gist.github.com/brimston3/2be168bb423c83b0f469c0be56e66d31

##########
File path: examples/celix-examples/services_example_cxx/src/DynamicConsumerBundleActivator.cc
##########
@@ -0,0 +1,159 @@
+/*
+ * 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 <utility>
+
+#include "celix/IShellCommand.h"
+#include "celix/BundleActivator.h"
+#include "examples/ICalc.h"
+
+namespace /*anon*/ {
+
+    class DynamicConsumer {
+    public:
+        explicit DynamicConsumer(int _id) : id{_id} {}
+
+        ~DynamicConsumer() {
+            stop();
+        }
+
+        void start() {
+            std::lock_guard<std::mutex> lck{mutex};
+            calcThread = std::thread{&DynamicConsumer::run, this};
+        }
+
+        void stop() {
+            active = false;
+            if (calcThread.joinable()) {
+                calcThread.join();
+                auto msg = std::string{"Destroying consumer nr "} + std::to_string(id) + "\n";
+                std::cout << msg;
+            }
+        }
+
+        void setCalc(std::shared_ptr<examples::ICalc> _calc, const std::shared_ptr<const celix::Properties>& props) {
+            std::lock_guard<std::mutex> lck{mutex};
+            if (_calc) {
+                calc = std::move(_calc);
+                svcId = props->getAsLong(celix::SERVICE_ID, -1);
+                ranking = props->getAsLong(celix::SERVICE_RANKING, 0);
+
+                std::string msg = "Setted calc svc for consumer nr " +  std::to_string(id);
+                msg += ". svc id is  "  + std::to_string(svcId);
+                msg += ", and ranking is " + std::to_string(ranking);
+                msg += "\n";
+                std::cout << msg;
+            } else {
+                std::string msg = "Resetting calc svc for consumer " + std::to_string(id)  + " to nullptr\n";
+                std::cout << msg;
+
+                calc = nullptr;
+                svcId = -1;
+                ranking = 0;
+            }
+        }
+    private:
+        void run() {
+            std::unique_lock<std::mutex> lck{mutex,  std::defer_lock};
+            int count = 1;
+            while (active) {
+                lck.lock();
+                auto localCalc = calc;
+                lck.unlock();
+
+                /*
+                 * note it is safe to use the localCalc outside a mutex,
+                 * because the shared_prt count will ensure the service cannot be unregistered while in use.
+                 */
+                if (localCalc) {
+                    auto msg = std::string{"Calc result is "} + std::to_string(localCalc->calc(count)) + "\n";

Review comment:
       Probably nicer to merge this with the cout, that way a std::string{} does not need to be constructed.

##########
File path: libs/framework/include/celix/BundleContext.h
##########
@@ -0,0 +1,580 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+#include <memory>
+#include <mutex>
+#include <thread>
+#include <cstdarg>
+
+#include "celix_bundle_context.h"
+
+#include "celix/ServiceRegistrationBuilder.h"
+#include "celix/UseServiceBuilder.h"
+#include "celix/TrackerBuilders.h"
+#include "celix/Bundle.h"
+#include "celix/Framework.h"
+
+#include "celix/dm/DependencyManager.h" //TODO, TBD include or forward declaration?
+
+namespace celix {
+
+    /**
+     * @brief The bundle context is used to interact with the Celix framework.
+     *
+     * The bundle context represent a bundle and can be used to:
+     *  - Register, use and track services
+     *  - Install, start, stop and uninstall bundles (runtime)
+     *  - log on framework level
+     *  - Access bundles
+     *  - Get config properties
+     *
+     * @note Thread safe.
+     */
+    class BundleContext {

Review comment:
       Are the async methods planned for the future?

##########
File path: libs/framework/include/celix_bundle_activator.h
##########
@@ -139,75 +139,6 @@ celix_status_t celix_bundleActivator_destroy(void *userData, celix_bundle_contex
 
 #ifdef __cplusplus
 }
-
-
-/**
- * This macro generates the required bundle activator functions for C++.
- * This can be used to more type safe bundle activator entries.
- *
- * The macro will create the following bundle activator functions:
- * - bundleActivator_create which allocates a pointer to the provided type.
- * - bundleActivator_start/stop which will call the respectively provided typed start/stop functions.
- * - bundleActivator_destroy will free the allocated for the provided type.
- *
- * @param type The activator type (e.g. 'ShellActivator'). A type which should have a constructor with a single arugment of std::shared_ptr<DependencyManager>.
- */
-#define CELIX_GEN_CXX_BUNDLE_ACTIVATOR(actType)                                                                        \

Review comment:
       Is there any transition macro possible to define to ease refactoring cost for users that have been using this?

##########
File path: libs/utils/src/filter.c
##########
@@ -58,11 +58,15 @@ void filter_destroy(celix_filter_t * filter) {
 static celix_filter_t * filter_parseFilter(char * filterString, int * pos) {
     celix_filter_t * filter;
     filter_skipWhiteSpace(filterString, pos);
-    if (filterString[*pos] != '(') {
+    if (filterString[*pos] == '\0') {
+        //empty filter
+        fprintf(stderr, "Filter Error: Cannot create LDAP filter from an empty string.\n");
+        return NULL;
+    } else if (filterString[*pos] != '(') {
         fprintf(stderr, "Filter Error: Missing '(' in filter string '%s'.\n", filterString);
         return NULL;
     }
-    (*pos)++;
+    (*pos)++; //eat '('

Review comment:
       :+1: 

##########
File path: libs/utils/src/properties.c
##########
@@ -346,7 +346,7 @@ void celix_properties_store(celix_properties_t *properties, const char *filename
 
 celix_properties_t* celix_properties_copy(const celix_properties_t *properties) {
     celix_properties_t *copy = celix_properties_create();
-    if (copy != NULL) {
+    if (properties != NULL) {

Review comment:
       And we've not had bugs pop up? Shocking.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] pnoltes commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r579829832



##########
File path: examples/celix-examples/services_example_cxx/src/DynamicConsumerBundleActivator.cc
##########
@@ -0,0 +1,159 @@
+/*
+ * 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 <utility>
+
+#include "celix/IShellCommand.h"
+#include "celix/BundleActivator.h"
+#include "examples/ICalc.h"
+
+namespace /*anon*/ {
+
+    class DynamicConsumer {
+    public:
+        explicit DynamicConsumer(int _id) : id{_id} {}
+
+        ~DynamicConsumer() {
+            stop();
+        }
+
+        void start() {
+            std::lock_guard<std::mutex> lck{mutex};
+            calcThread = std::thread{&DynamicConsumer::run, this};
+        }
+
+        void stop() {
+            active = false;
+            if (calcThread.joinable()) {
+                calcThread.join();
+                auto msg = std::string{"Destroying consumer nr "} + std::to_string(id) + "\n";
+                std::cout << msg;
+            }
+        }
+
+        void setCalc(std::shared_ptr<examples::ICalc> _calc, const std::shared_ptr<const celix::Properties>& props) {
+            std::lock_guard<std::mutex> lck{mutex};
+            if (_calc) {
+                calc = std::move(_calc);
+                svcId = props->getAsLong(celix::SERVICE_ID, -1);
+                ranking = props->getAsLong(celix::SERVICE_RANKING, 0);
+
+                std::string msg = "Setted calc svc for consumer nr " +  std::to_string(id);
+                msg += ". svc id is  "  + std::to_string(svcId);
+                msg += ", and ranking is " + std::to_string(ranking);
+                msg += "\n";
+                std::cout << msg;
+            } else {
+                std::string msg = "Resetting calc svc for consumer " + std::to_string(id)  + " to nullptr\n";
+                std::cout << msg;
+
+                calc = nullptr;
+                svcId = -1;
+                ranking = 0;
+            }
+        }
+    private:
+        void run() {
+            std::unique_lock<std::mutex> lck{mutex,  std::defer_lock};
+            int count = 1;
+            while (active) {
+                lck.lock();
+                auto localCalc = calc;
+                lck.unlock();
+
+                /*
+                 * note it is safe to use the localCalc outside a mutex,
+                 * because the shared_prt count will ensure the service cannot be unregistered while in use.
+                 */
+                if (localCalc) {
+                    auto msg = std::string{"Calc result is "} + std::to_string(localCalc->calc(count)) + "\n";

Review comment:
       Merged with cout. Although I did not use this originally because print line messages with cout can get mangled if multiple threads are using cout. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] pnoltes commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r579829613



##########
File path: bundles/shell/shell/src/Shell.cc
##########
@@ -0,0 +1,284 @@
+/*
+ * 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 <memory>
+#include <unordered_map>
+
+#include "celix/BundleActivator.h"
+#include "celix/IShellCommand.h"
+#include "std_commands.h"
+#include "celix_shell_command.h"
+#include "celix_log_service.h"
+#include "celix_shell_command.h"
+#include "celix_shell.h"
+
+namespace celix {
+
+    class Shell {
+    public:
+
+        /**
+         * Try to execute a command using the provided command line.
+         */
+        celix_status_t executeCommand(const char *commandLine, FILE *out, FILE *err) {
+            std::vector<std::string> tokens = tokenize(commandLine);
+            if (tokens.empty()) {
+                fprintf(err, "Invalid commandLine '%s'\n", commandLine);
+                return CELIX_ILLEGAL_ARGUMENT;
+            }
+
+            Entry entry = findCommand(tokens[0], err);
+            if (entry.cCommand) {
+                 bool successful = entry.cCommand->executeCommand(entry.cCommand->handle, commandLine, out, err);
+                 return successful ? CELIX_SUCCESS : CELIX_SERVICE_EXCEPTION;
+            } else if (entry.cxxCommand) {
+                std::vector<std::string> arguments{};
+                auto start = ++tokens.begin();
+                arguments.insert(arguments.begin(), start, tokens.end());
+                entry.cxxCommand->executeCommand(commandLine, arguments, out, err);

Review comment:
       Agree, this has been updated




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] Oipo commented on pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
Oipo commented on pull request #310:
URL: https://github.com/apache/celix/pull/310#issuecomment-783276356


   Looked at design, no remarks.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] pnoltes commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r579857348



##########
File path: libs/framework/include/celix/BundleContext.h
##########
@@ -0,0 +1,580 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+#include <memory>
+#include <mutex>
+#include <thread>
+#include <cstdarg>
+
+#include "celix_bundle_context.h"
+
+#include "celix/ServiceRegistrationBuilder.h"
+#include "celix/UseServiceBuilder.h"
+#include "celix/TrackerBuilders.h"
+#include "celix/Bundle.h"
+#include "celix/Framework.h"
+
+#include "celix/dm/DependencyManager.h" //TODO, TBD include or forward declaration?
+
+namespace celix {
+
+    /**
+     * @brief The bundle context is used to interact with the Celix framework.
+     *
+     * The bundle context represent a bundle and can be used to:
+     *  - Register, use and track services
+     *  - Install, start, stop and uninstall bundles (runtime)
+     *  - log on framework level
+     *  - Access bundles
+     *  - Get config properties
+     *
+     * @note Thread safe.
+     */
+    class BundleContext {

Review comment:
       One of the nice thinks about the builder api is that you can combine a lot functionality in a single builder (very concise). 
   For service registration the sync and async API is combined in a single builder. 
   
   The default for BundleContext::registerService is to register and unregister the service async.
   This is possible because registerService used a std::shared_ptr for the service so it can keep the service "alive" until async unregistration is completed. 
   https://github.com/apache/celix/blob/feature/cxx_headers/libs/framework/include/celix/BundleContext.h#L72
   
   https://github.com/apache/celix/blob/feature/cxx_headers/libs/framework/include/celix/ServiceRegistrationBuilder.h#L144
   https://github.com/apache/celix/blob/feature/cxx_headers/libs/framework/include/celix/ServiceRegistrationBuilder.h#L167




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] codecov-io edited a comment on pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #310:
URL: https://github.com/apache/celix/pull/310#issuecomment-753673666


   # [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=h1) Report
   > Merging [#310](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=desc) (9f7349e) into [master](https://codecov.io/gh/apache/celix/commit/3bec2e0e1445a81dfe076ae725412323882cb74f?el=desc) (3bec2e0) will **increase** coverage by `0.79%`.
   > The diff coverage is `93.67%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/celix/pull/310/graphs/tree.svg?width=650&height=150&src=pr&token=JdsiThga8P)](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #310      +/-   ##
   ==========================================
   + Coverage   70.14%   70.93%   +0.79%     
   ==========================================
     Files         154      172      +18     
     Lines       30449    31442     +993     
   ==========================================
   + Hits        21357    22304     +947     
   - Misses       9092     9138      +46     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ubsub/pubsub\_spi/src/pubsub\_interceptors\_handler.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2ludGVyY2VwdG9yc19oYW5kbGVyLmM=) | `48.93% <0.00%> (ø)` | |
   | [...ubsub/pubsub\_utils/src/pubsub\_serializer\_handler.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfc2VyaWFsaXplcl9oYW5kbGVyLmM=) | `90.95% <0.00%> (ø)` | |
   | [bundles/shell/shell/src/c\_shell.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-YnVuZGxlcy9zaGVsbC9zaGVsbC9zcmMvY19zaGVsbC5j) | `85.79% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/Component.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Db21wb25lbnQuaA==) | `72.72% <ø> (ø)` | |
   | [libs/framework/include/celix/dm/ProvidedService.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2UuaA==) | `100.00% <ø> (ø)` | |
   | [.../framework/include/celix/dm/ProvidedService\_Impl.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9Qcm92aWRlZFNlcnZpY2VfSW1wbC5o) | `97.05% <ø> (-0.17%)` | :arrow_down: |
   | [...ibs/framework/include/celix/dm/ServiceDependency.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeC9kbS9TZXJ2aWNlRGVwZW5kZW5jeS5o) | `92.30% <ø> (ø)` | |
   | [libs/framework/include/celix\_bundle\_context.h](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvaW5jbHVkZS9jZWxpeF9idW5kbGVfY29udGV4dC5o) | `100.00% <ø> (ø)` | |
   | [libs/framework/src/celix\_log.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2xvZy5j) | `89.39% <ø> (ø)` | |
   | [libs/framework/src/dm\_component\_impl.c](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2RtX2NvbXBvbmVudF9pbXBsLmM=) | `81.08% <ø> (+0.24%)` | :arrow_up: |
   | ... and [52 more](https://codecov.io/gh/apache/celix/pull/310/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=footer). Last update [3bec2e0...9f7349e](https://codecov.io/gh/apache/celix/pull/310?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [celix] pnoltes commented on a change in pull request #310: Adds initial impl for the C++ headers only wrappers

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #310:
URL: https://github.com/apache/celix/pull/310#discussion_r579830023



##########
File path: libs/framework/include/celix/Trackers.h
##########
@@ -0,0 +1,783 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+#include <memory>
+#include <mutex>
+#include <atomic>
+#include <cassert>
+#include <set>
+#include <unordered_map>
+
+#include "celix_utils.h"
+#include "celix/Properties.h"
+#include "celix/Utils.h"
+#include "celix/Bundle.h"
+#include "celix/Constants.h"
+#include "celix/Filter.h"
+#include "celix_bundle_context.h"
+
+namespace celix {
+
+
+    /**
+     * @brief The tracker state.
+     */
+    enum class TrackerState {
+        OPENING,
+        OPEN,
+        CLOSING,
+        CLOSED
+    };
+
+    /**
+     * @brief The AbstractTracker class is the base of all C++ Celix trackers.
+     *
+     * It defines how trackers are closed and manages the tracker state.
+     *
+     * This class can be used to create a vector of different (shared ptr)
+     * trackers (i.e. ServiceTracker, BundleTracker, MetaTracker).
+     *
+     * AbstractTracker <-----------------------------------
+     *  ^                               |                |
+     *  |                               |                |
+     *  GenericServiceTracker      BundleTracker    MetaTracker
+     *  ^
+     *  |
+     *  ServiceTracker<I>
+     *
+     * @note Thread safe.
+     */
+    class AbstractTracker {
+    public:
+        explicit AbstractTracker(std::shared_ptr<celix_bundle_context_t> _cCtx) :
+            cCtx{std::move(_cCtx)} {}
+
+        virtual ~AbstractTracker() noexcept = default;
+
+        /**
+         * @brief Check if the tracker is open (state == OPEN)
+         */
+        bool isOpen() const {
+            std::lock_guard<std::mutex> lck{mutex};
+            return state == TrackerState::OPEN;
+        }
+
+        /**
+         * @brief Get the current state of the tracker.
+         */
+        TrackerState getState() const {
+            std::lock_guard<std::mutex> lck{mutex};
+            return state;
+        }
+
+        /**
+         * @brief Close the tracker (of the state is not CLOSED or CLOSING).
+         *
+         * This will be done sync so then the close() method return the
+         * tracker is closed and all the needed callbacks have been called.
+         */
+        void close() {
+            long localTrkId = -1;
+            {
+                std::lock_guard<std::mutex> lck{mutex};
+                if (state == TrackerState::OPEN || state == TrackerState::OPENING) {
+                    //not yet closed
+                    state = TrackerState::CLOSING;
+                    localTrkId = trkId;
+                    trkId = -1;
+                }
+            }
+            if (localTrkId >= 0) {
+                celix_bundleContext_stopTracker(cCtx.get(), localTrkId);
+                {
+                    std::lock_guard<std::mutex> lck{mutex};
+                    state = TrackerState::CLOSED;
+                }
+            }
+        }
+
+        /**
+         * @brief Open the tracker (if the state is not OPEN or OPENING).
+         *
+         * This is done async, meaning that the actual opening of the tracker will be
+         * done a a Celix event processed on the Celix event thread.
+         *
+         * @throws celix::Exception
+         */
+        virtual void open() {}

Review comment:
       Nope, should be a pure virtual. updated. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org