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

Move type checking to a static_assert #38

Merged
merged 1 commit into from
Jun 10, 2015
Merged

Conversation

esteve
Copy link
Member

@esteve esteve commented Jun 10, 2015

VS2015 still fails to compile when callbacks have only two arguments, as it insists on checking for a non-existent third argument (causing an index out of bounds error on std::tuple_element).

This change just moves the check to a static_assert in the body of the method.

@dirk-thomas @tfoote @wjwwood

@esteve esteve added the in progress Actively being worked on (Kanban column) label Jun 10, 2015
@esteve
Copy link
Member Author

esteve commented Jun 10, 2015

While trying to fix this, I came up with the following code for checking several conditions on a single std::enable_if, maybe with a bit more of work we can move the check to the template section instead of static_assert back, but at this point I don't think it's worth us spending more time on this.

template <typename... T>
struct All : std::true_type {};

template <typename First, typename... Last>
struct All<First, Last...> :
std::conditional<First::value, All<Last...>, std::false_type>::type
{};

template<
  typename FunctorT,
  typename std::enable_if<All<
    std::is_same<
      typename function_traits<FunctorT>::template argument_type<0>,
      int>,
    typename std::conditional<function_traits<FunctorT>::arity == 1, std::true_type, std::false_type>::type
  >::value>::type * = nullptr
>
void
test1(FunctorT callback) {}

I also think we could whip up something with std::forward, but as I said, there's no real benefit spending more time on this. If anyone feels like giving it a chance, just let me know.

@wjwwood
Copy link
Member

wjwwood commented Jun 10, 2015

I agree, if it works around the issue for now then we should just move on. Thanks for taking time to look into it.

@esteve
Copy link
Member Author

esteve commented Jun 10, 2015

Thanks @wjwwood for your review. I'm merging this so we can continue working on Windows.

esteve added a commit that referenced this pull request Jun 10, 2015
Move type checking to a static_assert
@esteve esteve merged commit 17c07b8 into master Jun 10, 2015
@esteve esteve removed the in progress Actively being worked on (Kanban column) label Jun 10, 2015
@esteve esteve deleted the windows-sfinae-workaround branch June 10, 2015 17:58
@wjwwood
Copy link
Member

wjwwood commented Jun 10, 2015

@esteve FYI there was a style issue with this pr:

http://54.183.26.131:8080/job/ros2_batch_ci_linux/141/testReport/junit/rclcpp/uncrustify/__include_rclcpp_node_hpp/

Can you just push an update to fix that directly?

@esteve
Copy link
Member Author

esteve commented Jun 10, 2015

Fixed, thanks for noticing @wjwwood

On Wed, Jun 10, 2015 at 2:15 PM, William Woodall notifications@github.com
wrote:

@esteve https://github.com/esteve FYI there was a style issue with this
pr:

http://54.183.26.131:8080/job/ros2_batch_ci_linux/141/testReport/junit/rclcpp/uncrustify/__include_rclcpp_node_hpp/

Can you just push an update to fix that directly?


Reply to this email directly or view it on GitHub
#38 (comment).

mauropasse pushed a commit to mauropasse/rclcpp that referenced this pull request Feb 11, 2021
nnmm pushed a commit to ApexAI/rclcpp that referenced this pull request Jul 9, 2022
fix unknownEvaluationOrder warning
DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this pull request Aug 5, 2022
* First implementation of writer

* Extract storage interface and sqlite3 implementation

* Add test for sqlite storage

* Split main() and rosbag2::record()

* Add close() method to Storage

* Add getMessage() method and refactor test

* Refactor SqliteStorage constructor and open()

* Add linters

* Fix uncrustify

* Fix cpplint

* Specify test working directory

* Better error handling

* Use gmock matchers for assertions

* Add test fixture for SqliteStorage tests

* Extract message retrieval in tests into separate method

* Add integration test for rosbag2::record()

* Add ignore files for empty packages

* Introduce create() method and refactor open()

* Use shared pointer of Storage instead of SqliteStorage

* Remove getDatabaseHandle() method

* Fix uncrustify

* Improve storage interface and add storage factory

* Remove need of sleep() from integration test by usage of std::future

* Move deletion of test database from fixture constructor to destructor

* Use sqlite3 directly in intergration test instead of own sqlite wrapper

* Move rosbag2::record() into Rosbag2 class

* Use the test name as database file name

* Add build instructions to README

* ros2GH-37 Rename camelCase methods to snake_case

* Use common test fixture

* Add RAII wrapper for sqlite API

* Mock away sqlite from sqlite_storage test

* Use more reasonable assert

* Add test

* Add virtual destructor to WritableStorage

* Use file_name instead of database_name in StorageFactory

* Implement saving of test files in a tmp directory for linux/Mac

* Try to implement saving of test files in a tmp directory for Windows

* Write and use proper gmock SqliteWrappe mock

* Refactor integration test and get rid of promise/future where possible

* Throw exception in resource aquisition constructors

* Make SqliteWrapper destructor virtual

* Refactor test fixture and update SqliteWrapper mock

* Fix warning when moving a temporary object

* ros2GH-38 Refactor integration test

* ros2GH-38 Get rid of superfluous string constructor in emplace_back()

* ros2GH-38 Assert also execute_query() argument in sqlite_storage_test

* ros2GH-38 add StorageFactory test

* ros2GH-38 Refactor rosbag2 Test Fixture

* ros2GH-40 Add first implementation of a rosbag reader and publisher

* ros2GH-40 Add StorageFactory test when reading non-existing file

* ros2GH-40 Fix uncrustify

* ros2GH-40 Minor cleanup of CMakeLists

* ros2GH-40 Wrap sqlite statements

* ros2GH-40 Remove superfluous import

* ros2GH-40 Use better include

* ros2GH-40 Add play integration test

* ros2GH-40 Fix Warning when moving a temporary object in reading

* ros2GH-40 Initialize database pointer to nullpointer

* ros2GH-40 Fix reader integration test

* ros2GH-40 Polish storage wrapper

* Revert "ros2GH-40: Wrap sqlite statements"

* ros2GH-38 Fix Test Fixture after rebase

* ros2GH-38 Refactor read_integration_test and refix Windows conversion warning

* ros2GH-38 Add StorageFactory test

* Simplify storage factory test

* ros2GH-38 Try to fix flaky test

* ros2GH-38 Move rclcpp::shutdown() at the end

* ros2GH-41 Fix windows warning due to virtual explicit operator bool

* ros2GH-41 Use sqlite3 vendor package in rosbag2

* ros2GH-41 Stop linking tests to sqlite

* ros2GH-41 Fix test fixture on Windows

* ros2GH-41 Cleanup test fixture includes

* ros2GH-41 Print test database name

* ros2GH-41 Correctly determine temp dir on Windows

* ros2GH-41 Show error message on sqlite_open failure

* ros2GH-41 Actually create temp dir on Windows

* ros2GH-41 Fix bool conversion warning in VS2015 build

* Fix CMakeLists.txt after rebase

* ros2GH-40 Implement workouround to fix flaky test

* Update package.xml

* Add gtest test dependencies to package.xml

* ros2GH-40 Move to sqlite3_storage_plugin folder

- The separation into the intended structure and plugin apis is not
  there yet. However, most code will stay in the storage plugin for
  sqlite3 file.
- Proper separation of this code into storage plugin and rosbag layer
  will be done in bosch-io/aos-rosbag2#5.

* ros2GH-40 Add TODO comments and small cleanup
DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this pull request Aug 5, 2022
* rosbag2_transport package with python interface

* use cpp for python extension

* use rosbag2_transport cpp API

* use rosbag2_transport API in cli

* linters

* ros2GH-25 Rename target librosbag2 to rosbag2

CMake already prepends libraries with `lib`, so the old name resulted
in `liblibrosbag2`

* ros2GH-21 Initial call of rosbag2.record() from rosbag2_transport

* ros2GH-21 Add missing copyright header

* ros2GH-21 Cleanup clang tidy issues

* ros2GH-21 Remove rclcpp dependency from rosbag2

* ros2GH-21 Wire rosbag play into CLI

* ros2GH-21 Add missing test_depend in rosbag2_transport package.xml

* ros2GH-21 Unify name of python import

* ros2GH-21 Enable -a in CLI, show help on wrong args

* ros2GH-85 Introduce topic and type struct for readability

* ros2GH-85 Do not export sqlite3 as dependency from default plugins

- not referenced in header, therefore unnecessary

* ros2GH-85 Move rosbag2 except typesupport to rosbag2_transport

* ros2GH-85 Add rosbag2 wrapper

* ros2GH-85 Change signature of create_topic to take TopicWithType

* ros2GH-85 Use rosbag2 in rosbag2_transport

- Don't link against rosbag2_storage anymore

* ros2GH-84 Cleanup package.xmls and CMakeLists everywhere

* ros2GH-21 Add missing init() and shutdown() in record

* ros2GH-85 Fix Windows build

* ros2GH-85 Add visibility control to rosbag2

* ros2GH-85 Cleanup and documentation

* ros2GH-87 Add test package rosbag2_tests

* ros2GH-87 [WIP] Add first working prototype of an end-to-end test

* ros2GH-87 Use test_msgs instead of std_msgs/String in end-to-end test

* ros2GH-87 Use SIGTERM instead of SIGKILL and refactor test

* ros2GH-87 Make end-to-end test work on Windows

* ros2GH-87 Fix uncrustify

* ros2GH-87 Refactor end-to-end test fixture

* ros2GH-21 Extend transport python module interface

The python interface should accept all options that can be passed to rosbag2_transport

* ros2GH-87 Fix test fixture for Windows

* ros2GH-87 Refactor test fixture

* ros2GH-87 Separate record from play end-to-end test

* ros2GH-87 Make record end-to-end test work

* ros2GH-87 Publish before recording to create topic

* ros2GH-87 Fix record all on Windows

* ros2GH-87 Check for topics instead of all

* ros2GH-87 Wait until rosbag record opened database

* ros2GH-87 Delete directory recursively

* ros2GH-87 Delete directories recursively on Linux

* ros2GH-87 Reset ROS_DOMAIN_ID to protect against concurrent tests

* ros2GH-89 Make rosbag2 interfaces virtual and add explicit open() method

This allows downstream packages (e.g. rosbag2_transport) to mock these
interfaces in tests.

* ros2GH-87 Improve test and refactoring

* ros2GH-87 Minor refactoring to increase test readability

* ros2GH-87 Fix environmental variable behaviour on Mac

* ros2GH-87 Fix Windows build

* ros2GH-89 Use mock reader and writer in rosbag2_transport tests

* ros2GH-87 Add play end_to_end test

* ros2GH-87 Improvements of test

* ros2GH-87 Fix Windows build

* ros2GH-89 Cleanup: small documentation fixes.

* ros2GH-89 [WIP] Test if Writer and Reader work with class visibility

* ros2GH-87 Stabilize rosbag2_play test

* ros2GH-87 Minor refactoring of tests

* ros2GH-87 Rename end to end tests

* add license agreement

* ros2GH-89 Simplification of writing to in-memory storage

* ros2GH-89 Stabilize transport tests

* ros2GH-87 Refactoring of tests

- Extract temporary file handling
- Extract subscription management

* ros2GH-87 Add pytest cache to gitignore

* ros2GH-87 Refactoring of play test

- Extract Publisher manager

* ros2GH-87 Extract record test fixture for readability

* ros2GH-89 Refactor transport tests

- Use subscription and publisher manager just as e2e tests
- Use options in recording

* ros2GH-89 Use temporary directory fixture in sqlite tests

* ros2GH-89 Conform to naming standard for tests

* ros2GH-89 Prevent burst publishing of all messages

- Improves test stability

* ros2GH-89 Improve play stability

- Sometimes the first message is lost (discovery)

* ros2GH-25 Fix package.xmls

* Consistently use project name in CMakeLists

* Minor cleanup

- make rosbag2_transport description more expressive
- hide unnecessary methods in typesupport_helpers
- fix incorrect logging in tests
- minor cleanup

* Change name of nodes in rosbag2_transport

* Cleanup folder structure in rosbag2_storage and rosbag2_tests

- use src/<package_name>/ and test/<package_name>/ folders everywhere
- harmonises with all other packages
- results in better header guards

* Export sqlite3 dependency as package dependency

* Create node in Rosbag2Transport always

* Only hold one node in rosbag2_transport

* Move all duplicate files to common package

* Adapt namespacing in test commons package

- use "using namespace" declaratives for tests
- use package name as namespace

* Replace "Waiting for messages..." message

* ros2GH-25 rename rosbag2_test_commons -> rosbag2_test_common

* ros2GH-25 Overwrite already existing test.bag when recording

This is a temporary solution and will be handled properly once a
file path can be passed via the cli.

* ros2GH-25 Cleanups

- Log every subscription
- move all dependencies onside BUILD_TESTING for rosbag2_test_common

* fix cmake typo for test_common

* Remove superfluous loop in rosbag2 transport

* Delete superfluous test_msgs dependency

* Add rclcpp to test dependencies

- Apparently ament_export_dependencies does not work in rosbag2_test_common

* Fix rosbag2 node test

- Clock topic is no longer present on all nodes
- Remove assumptions on foreign ros topics

* Fix dependencies by exporting them explicitly
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