Conversation
a) can't combine filters (like `| EVFILT_VNODE`) b) wrong `EV_CLEAR` usage c) wrong `NOTE_xxx` usage
# Conflicts: # .gitmodules # submodules/spdlog # submodules/toml11
init.d/ocvsmd.toml
Outdated
| # Supported formats: | ||
| # - 'udp://<ip4>' | ||
| # - 'socketcan:<can_device>' | ||
| # - 'socketcan:<can_device>' (linux only) |
There was a problem hiding this comment.
Technically, SocketCAN is available not only on Linux -- it's also available in at least Zephyr and NuttX
src/daemon/engine/CMakeLists.txt
Outdated
| if (${PLATFORM_OS_TYPE} STREQUAL "linux") | ||
| add_library(canard | ||
| ${submodules_dir}/libcanard/libcanard/canard.c | ||
| ) | ||
| target_include_directories(canard | ||
| INTERFACE SYSTEM ${submodules_dir}/libcanard/libcanard | ||
| ) | ||
| endif () |
There was a problem hiding this comment.
This seems to imply that we cannot use CAN in macOS at all, but that's only true because we only support SocketCAN. One could easily add a driver for SLCAN or gs-usb and use CAN on any OS in this manner, so this exclusion is misleading here.
src/daemon/engine/CMakeLists.txt
Outdated
| target_link_libraries(ocvsmd_engine | ||
| PUBLIC canard | ||
| ) |
There was a problem hiding this comment.
I suggest moving this block outside of the conditional to avoid the false implication that CAN requires Linux
|
|
||
| #ifdef __linux__ | ||
| # include <libcyphal/transport/can/can_transport.hpp> | ||
| #endif |
pavel-kirienko
left a comment
pavel-kirienko
left a comment
There was a problem hiding this comment.
I think these conditionals on __linux__ are misleading. How about we add a config macro like OCVSMD_USE_CAN, and populate it from CMake, such that it is enabled by default only on Linux, and we add a comment explaining that one day we may add gs-usb or something to make it work on other platforms?
In the next occasion I'll get rid of |
4 participants
kqueuefixes (from thedemosrepo).EINTR"error" from::kevent&epoll_wait).