Feature/nanobind submodule #1556
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| { | ||
| std::string Rec_Data = "No Reception"; | ||
| long long rcv_time = 12345; | ||
| auto success = eCAL::CSubscriber::ReceiveBuffer(Rec_Data, &rcv_time, 500); |
There was a problem hiding this comment.
warning: Value stored to 'success' during its initialization is never read [clang-analyzer-deadcode.DeadStores]
auto success = eCAL::CSubscriber::ReceiveBuffer(Rec_Data, &rcv_time, 500);
^Additional context
ecal/core/include/ecal/ecal_nb_subscriber.h:17: Value stored to 'success' during its initialization is never read
auto success = eCAL::CSubscriber::ReceiveBuffer(Rec_Data, &rcv_time, 500);
^| @@ -1,5 +1,164 @@ | |||
| #include <nanobind/nanobind.h> | |||
There was a problem hiding this comment.
warning: 'nanobind/nanobind.h' file not found [clang-diagnostic-error]
#include <nanobind/nanobind.h>
^
| .def("get_datatype_information", &eCAL::CSubscriber::GetDataTypeInformation) | ||
| .def("dump", &eCAL::CSubscriber::Dump); | ||
|
|
There was a problem hiding this comment.
We need only one class "Subscriber" which wraps the CNBSubscriber class, but then it wraps all methods.
lang/python/nanobind_core/main.cpp
Outdated
| m.def("get_version", [](int* nb_major, int* nb_minor, int* nb_patch) | ||
| { return eCAL::GetVersion(nb_major, nb_minor, nb_patch); }); | ||
| // m.def("initialize", [](int nb_argc_, char *nb_argv_, const char* nb_unit_name_, unsigned int nb_components_) |
There was a problem hiding this comment.
this function should return return all three values instead of taking them as a pointer.
in python [major, minor, patch] = get_version().
so you can use nb::tuple to return a tuple.
https://nanobind.readthedocs.io/en/latest/api_core.html#_CPPv4N8nanobind5tupleE
lang/python/nanobind_core/main.cpp
Outdated
| m.def("get_version_string", []() { return eCAL::GetVersionString(); }); | ||
| m.def("get_version_date", []() { return eCAL::GetVersionDateString(); }); | ||
| m.def("set_unitname", [](const char* nb_unit_name) { return eCAL::SetUnitName(nb_unit_name); }); |
There was a problem hiding this comment.
you need here to take a std::string I think. you can then do return eCAL::SetUnitName(nb_unit_name.c_str())
|
|
||
| m.def("get_topics", [](std::unordered_map<std::string, eCAL::SDataTypeInformation>& nb_topic_info_map_) | ||
| { return eCAL::Util::GetTopics(nb_topic_info_map_); }); |
There was a problem hiding this comment.
with this function you probably need to return a map.
lang/python/nanobind_core/main.cpp
Outdated
| { return eCAL::Util::GetTopicNames(nb_topic_names_); }); | ||
| m.def("get_topic_datatype_info", [](const std::string& nb_topic_name_, eCAL::SDataTypeInformation& nb_topic_info_) | ||
| { return eCAL::Util::GetTopicDataTypeInformation(nb_topic_name_, nb_topic_info_); }); |
There was a problem hiding this comment.
this function also: take a std::string, return [bool, SDatatypeInformation]
ashariff-11
force-pushed
the
feature/nanobind-submodule
branch
from
068d552
to
00c01df
Compare
| #include <wrappers/datatypeinfo.h> | ||
| #include <wrappers/publisher.h> | ||
| #include <wrappers/server.h> | ||
| #include <wrappers/subscriber.h> |
There was a problem hiding this comment.
#include only modules/...
|
|
||
| //void AddSubscriberClassToModule(const nanobind::module_& module) {}; | ||
|
|
||
| NB_MODULE(nanobind_core, m) { |
There was a problem hiding this comment.
should be in modules/datatypeinfo.h
| .def_rw("descriptor", &eCAL::CNBDataTypeInformation::descriptor); | ||
|
|
||
| // Struct eCAL::SServiceResponse | ||
| nanobind::class_<eCAL::SServiceResponse>(m, "ServiceResponse") |
There was a problem hiding this comment.
move to file... /server.h
|
|
||
| namespace eCAL | ||
| { | ||
| CNBSrvClient::CNBSrvClient() : CServiceClient() { } |
There was a problem hiding this comment.
CNBSrvCLient -> CNBServiceClient
| { | ||
| public: | ||
| std::string name; | ||
| std::string encoding; |
There was a problem hiding this comment.
maybe: nanobind:string???
| CNBPublisher::CNBPublisher(const std::string& topic_name, const CNBDataTypeInformation& datainfo) : CPublisher(topic_name, convert(datainfo)) { } | ||
|
|
||
| bool CNBPublisher::WrapAddPubEventCB(eCAL_Publisher_Event event, nanobind::callable callback_) | ||
| { |
There was a problem hiding this comment.
Think about naming
|
|
||
| namespace eCAL | ||
| { | ||
| CNBSrvServer::CNBSrvServer() : CServiceServer() { } |
There was a problem hiding this comment.
CNBSrvServer -> CNBServiceServer
| #include <string> | ||
| #include <cstddef> | ||
| #include <ecal/ecal_types.h> | ||
|
|
There was a problem hiding this comment.
in this file, probably the only include you need is #include <nanobind/nanobind.h> (for nanobind::module).
If then it doesn't compile, move the includes to module_subscriber.cpp.
Description
Related issues
Cherry-pick to