Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API: Introduce experimental namespace to mark new api as not yet stable. #1307

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

KerstinKeller
Copy link
Contributor

@KerstinKeller KerstinKeller commented Jan 16, 2024

Description

Introduce experimental namespace for measurement to mark new API as not yet stable.

Cherry-pick to

  • none

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

@@ -58,7 +58,7 @@ bool EcalPlay::LoadMeasurement(const std::string& path)
{
EcalPlayLogger::Instance()->info("Loading measurement...");

std::shared_ptr<eCAL::measurement::base::Reader> measurement(std::make_shared<eCAL::measurement::hdf5::Reader>());
std::shared_ptr<eCAL::experimental::measurement::base::Reader> measurement(std::make_shared<eCAL::experimental::measurement::hdf5::Reader>());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'measurement' of type 'std::shared_ptreCAL::experimental::measurement::base::Reader' can be declared 'const' [misc-const-correctness]

Suggested change
std::shared_ptr<eCAL::experimental::measurement::base::Reader> measurement(std::make_shared<eCAL::experimental::measurement::hdf5::Reader>());
std::shared_ptr<eCAL::experimental::measurement::base::Reader> const measurement(std::make_shared<eCAL::experimental::measurement::hdf5::Reader>());

@@ -134,7 +134,7 @@ namespace eCAL
bool operator!=(const IChannel& rhs) const { return !(operator==(rhs)); }

//virtual Entry<T> operator[](unsigned long long timestamp);
virtual Frame<T> operator[](const base::EntryInfo& entry)
virtual Frame<T> operator[](const experimental::measurement::base::EntryInfo& entry)
{
auto binary_entry = binary_channel[entry];
eCAL::message::Deserialize(binary_entry.message, message);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'message' is not a class, namespace, or enumeration [clang-diagnostic-error]

        eCAL::message::Deserialize(binary_entry.message, message);
              ^
Additional context

contrib/measurement/base/include/ecal/measurement/imeasurement.h:213: 'message' declared here

      mutable T message;
                ^

@@ -72,7 +72,7 @@ namespace eCAL
class OChannel
{
public:
OChannel(std::shared_ptr<base::Writer> meas_, std::string name_)
OChannel(std::shared_ptr<experimental::measurement::base::Writer> meas_, std::string name_)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: the parameter 'name_' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]

Suggested change
OChannel(std::shared_ptr<experimental::measurement::base::Writer> meas_, std::string name_)
OChannel(std::shared_ptr<experimental::measurement::base::Writer> meas_, const std::string& name_)

/**
* @brief Destructor
**/
virtual ~Reader();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [cppcoreguidelines-explicit-virtual-functions]

Suggested change
virtual ~Reader();
~Reader() override;

/**
* @brief Destructor
**/
virtual ~Writer();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [cppcoreguidelines-explicit-virtual-functions]

Suggested change
virtual ~Writer();
~Writer() override;

@@ -279,7 +279,7 @@ static PyObject* Meas_GetEntriesInfo(Meas *self, PyObject *args)
if (!PyArg_ParseTuple(args, "s", &channel_name))
return nullptr;

eCAL::measurement::base::EntryInfoSet entries;
eCAL::experimental::measurement::base::EntryInfoSet entries;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'entries' is not initialized [cppcoreguidelines-init-variables]

Suggested change
eCAL::experimental::measurement::base::EntryInfoSet entries;
eCAL::experimental::measurement::base::EntryInfoSet entries = 0;

@@ -327,7 +327,7 @@ static PyObject* Meas_GetEntriesInfoRange(Meas *self, PyObject *args)
if (!PyArg_ParseTuple(args, "sLL", &channel_name, &begin, &end))
return nullptr;

eCAL::measurement::base::EntryInfoSet entries;
eCAL::experimental::measurement::base::EntryInfoSet entries;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'entries' is not initialized [cppcoreguidelines-init-variables]

Suggested change
eCAL::experimental::measurement::base::EntryInfoSet entries;
eCAL::experimental::measurement::base::EntryInfoSet entries = 0;

@KerstinKeller KerstinKeller merged commit 710d8d3 into master Jan 17, 2024
14 checks passed
KerstinKeller added a commit that referenced this pull request Jan 18, 2024
* Doc: Downgraded sphinx related packages in requirements.txt (#1302)

* The most recent release of various sphinx related packages need Sphinx 5 or up, but our old sphinx-book-theme 0.3.3 requires Sphinx 4
For a proper solution see #1303

* CMake: Fixed Findqwt.cmake (#1306)

* [Python/clang] Add missing core functions (#1300)

Adds the missing core functions from ecal_core.h (GetVersion, IsInitialized, SetUnitName) to ecal_clang and the python wrapper.

* Measurement API: Introduce experimental namespace to mark new api as not yet stable. (#1307)

---------

Co-authored-by: Florian Reimold <[email protected]>
Co-authored-by: DownerCase <[email protected]>
@KerstinKeller KerstinKeller deleted the hotfix/experimental-measurement-namespace branch January 25, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants