search

iqrfsdk / clibdpa

IQRF DPA library for Linux and Windows
Apache License 2.0
0 stars 2 forks source link

Minor issue: DPA-example does not clean up the IChannel resource. #35

Closed MelvinWM closed 5 years ago

MelvinWM commented 5 years ago

In the DPA-example at line https://github.com/iqrfsdk/clibdpa/blob/master/DpaExamples/Example.cpp#L52 , the IChannel resource does not appear to be cleaned up, since the constructor of DpaHandler2 does not take ownership of it, and thus it ends up not being deleted. It would be good if the example cleaned up resources properly, as it seeks to do (see for instance https://github.com/iqrfsdk/clibdpa/blob/master/DpaExamples/Example.cpp#L157 ).

fmikulu commented 5 years ago

Examples are "just in examples quality" illustrating mainly usage of DPA and weren't reviewed properly with respect of perfect cleaning (memleaks). If you find something wrong there, don't hesitate to do pull request.

BTW clidpa is used by more complex project : https://gitlab.iqrf.org/open-source/iqrf-gateway-daemon

Maybe it can be an option for your project.

MelvinWM commented 5 years ago

Dear @fmikulu , thank you.

As far as I can quickly 'grep' in the project, IQRF Gateway Daemon does not seem to use either of the classes IqrfSpiChannel and IqrfCdcChannel, possibly using its own implementations for handling SPI as well as CDC instead of using clibdpa's implementations. See for instance https://gitlab.iqrf.org/open-source/iqrf-gateway-daemon/tree/master/src/IqrfSpi and https://gitlab.iqrf.org/open-source/iqrf-gateway-daemon/tree/master/src/IqrfCdc . Can you confirm this?

The class IqrfDpaChannel also has a comment about something reg. clibdpa here: https://gitlab.iqrf.org/open-source/iqrf-gateway-daemon/blob/master/src/IqrfDpa/IqrfDpaChannel.h#L7 .

The class IqrfDpa seems at a glance like a wrapper around DpaHandler2, and that DpaHandler2 is not used directly. Of course, IQRF Gateway Daemon may have very different needs from other kinds of client users of clibdpa, so its kind of usage of clibdpa may likewise vary.

fmikulu commented 5 years ago

We have three implementations of IF to IQRF Coordinator: clibspi, clibuart, clibcdc. The lib clibdpa is based upon them. This is designed to accept abstract IF IChannel and thus implemention based on one of cdc/spi/uart represented by IqrfSpiChannel and IqrfCdcChannel (uart not supported in this way).

Historically clidpa was there before IQRF Gateway Daemon (IQRFGD) and later we adapted it for IQRFGD but to keep possibility to use it outside IQRFGD for backward compatibility . You are right the classes IqrfSpiChannel and IqrfCdcChannel are not used in IQRFGD.

As we concentrate on IQRFGD we consider to stop clibdpa maintenance and just to keep it as it is now. I can admit that IQRFGD is too heavy for (OS-less) embedded one-purpose applications. In this case it is questionable if transaction processing of clibdpa is really necessary. If you are on OS (linux, win) then I would recommend to use IQRFGD.

fmikulu commented 5 years ago

BTW not all components of IQRFGD are required and it is very flexible to be adapted and configured for one purpose, even embedded to be part of other process if necessary.

MelvinWM commented 5 years ago

Dear @fmikulu , thank you very much, that helps a lot, especially which library/system to use. I will likely move our system over to IQRFGD, since the code I am developing that is using clibdpa is deployed on Raspberry Pi. I do not have any more questions, you are welcome to close this issue. Thank you again. Best regards.