Skip to content

allow to set custom serial speed on Linux #80534

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.

Sign up for GitHub

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

Merged
wfurt merged 4 commits into from
Jan 20, 2023
Merged

allow to set custom serial speed on Linux #80534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9eac73b
allow to set custom serial speed on Linux
wfurt Jan 12, 2023
4961dbe
fix compilation on alpine
wfurt Jan 12, 2023
278f95d
add entries for cross build
wfurt Jan 16, 2023
b949d32
Merge branch 'main' of https://github.com/dotnet/runtime into serial2
wfurt Jan 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions eng/native/tryrun.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ elseif(TARGET_ARCH_NAME MATCHES "^(armel|arm|armv6|arm64|loongarch64|riscv64|s39
set_cache_value(PTHREAD_CREATE_MODIFIES_ERRNO_EXITCODE 1)
set_cache_value(REALPATH_SUPPORTS_NONEXISTENT_FILES_EXITCODE 1)
set_cache_value(SEM_INIT_MODIFIES_ERRNO_EXITCODE 1)
set_cache_value(HAVE_TERMIOS2_EXITCODE 0)


if(ALPINE_LINUX)
Expand All @@ -128,6 +129,7 @@ elseif(TARGET_ARCH_NAME MATCHES "^(armel|arm|armv6|arm64|loongarch64|riscv64|s39
set_cache_value(UNGETC_NOT_RETURN_EOF 0)
set_cache_value(HAVE_COMPATIBLE_ILOGBNAN 1)
set_cache_value(HAVE_FUNCTIONAL_PTHREAD_ROBUST_MUTEXES_EXITCODE 0)
set_cache_value(HAVE_TERMIOS2_EXITCODE 1)
elseif(ILLUMOS)
set_cache_value(GETPWUID_R_SETS_ERRNO_EXITCODE 1)
set_cache_value(HAVE_COMPATIBLE_ACOS_EXITCODE 1)
Expand All @@ -146,6 +148,7 @@ elseif(TARGET_ARCH_NAME MATCHES "^(armel|arm|armv6|arm64|loongarch64|riscv64|s39
set_cache_value(HAVE_SET_MAX_VARIABLE 1)
set_cache_value(HAVE_FULLY_FEATURED_PTHREAD_MUTEXES 1)
set_cache_value(HAVE_FUNCTIONAL_PTHREAD_ROBUST_MUTEXES_EXITCODE 0)
set_cache_value(HAVE_TERMIOS2_EXITCODE 1)
elseif (TIZEN)
set_cache_value(HAVE_FUNCTIONAL_PTHREAD_ROBUST_MUTEXES_EXITCODE 0)
endif()
Expand Down
1 change: 1 addition & 0 deletions src/native/libs/Common/pal_config.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@
#cmakedefine01 HAVE_MAKEDEV_FILEH
#cmakedefine01 HAVE_MAKEDEV_SYSMACROSH
#cmakedefine01 HAVE_GETGRGID_R
#cmakedefine01 HAVE_TERMIOS2

// Mac OS X has stat64, but it is deprecated since plain stat now
// provides the same 64-bit aware struct when targeting OS X > 10.5
Expand Down
16 changes: 12 additions & 4 deletions src/native/libs/System.IO.Ports.Native/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,26 @@
project(System.IO.Ports.Native C)

set(PORTS_SOURCES
pal_termios.c
pal_serial.c
)

if (HAVE_TERMIOS2)
list (APPEND PORTS_SOURCES pal_termios2.c)
endif ()


if (GEN_SHARED_LIB)
add_library(System.IO.Ports.Native
SHARED
pal_termios.c
pal_serial.c
${PORTS_SOURCES}
${VERSION_FILE_PATH}
)
endif()

add_library(System.IO.Ports.Native-Static
STATIC
pal_termios.c
pal_serial.c
${PORTS_SOURCES}
)

set_target_properties(System.IO.Ports.Native-Static PROPERTIES OUTPUT_NAME System.IO.Ports.Native CLEAN_DIRECT_OUTPUT 1)
Expand Down
14 changes: 10 additions & 4 deletions src/native/libs/System.IO.Ports.Native/pal_termios.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,13 +355,16 @@ int32_t SystemIoPortsNative_TermiosSetSpeed(intptr_t handle, int32_t speed)

if (brate == B0)
{
#if HAVE_IOSS_H
// Looks like custom speed out of POSIX. Let see if we can set it via specialized call.
#if HAVE_IOSS_H
brate = speed;
if (ioctl(fd, IOSSIOSPEED, &brate) != -1)
{
return speed;
}
#endif
#if HAVE_TERMIOS2
return SystemIoPortsNative_Termios2SetSpeed(fd, speed);
#endif
errno = EINVAL;
return -1;
Expand Down Expand Up @@ -518,7 +521,7 @@ int32_t SystemIoPortsNative_TermiosReset(intptr_t handle, int32_t speed, int32_t
brate = TermiosSpeed2Rate(speed);
if (brate == B0)
{
#if !HAVE_IOSS_H
#if !HAVE_IOSS_H && !HAVE_TERMIOS2
// We can try to set non-standard speed after tcsetattr().
errno = EINVAL;
return -1;
Expand All @@ -539,17 +542,20 @@ int32_t SystemIoPortsNative_TermiosReset(intptr_t handle, int32_t speed, int32_t
return -1;
}

#if HAVE_IOSS_H
if (speed && brate == B0)
{
#if HAVE_IOSS_H
// we have deferred non-standard speed.
brate = speed;
if (ioctl(fd, IOSSIOSPEED, &brate) == -1)
{
return -1;
}
}
#endif
#if HAVE_TERMIOS2
return SystemIoPortsNative_Termios2SetSpeed(fd, speed);
#endif
}

return 0;
}
2 changes: 2 additions & 0 deletions src/native/libs/System.IO.Ports.Native/pal_termios.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#include "pal_types.h"
#include "pal_compiler.h"

int SystemIoPortsNative_Termios2SetSpeed(int fd, int speed);

PALEXPORT int32_t SystemIoPortsNative_TermiosGetSignal(intptr_t fd, int32_t signal);
PALEXPORT int32_t SystemIoPortsNative_TermiosSetSignal(intptr_t fd, int32_t signal, int32_t set);
PALEXPORT int32_t SystemIoPortsNative_TermiosGetAllSignals(intptr_t fd);
Expand Down
31 changes: 31 additions & 0 deletions src/native/libs/System.IO.Ports.Native/pal_termios2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#include "pal_types.h"
#include "pal_utilities.h"
#include <asm/termbits.h>
#include <asm/ioctls.h>
#include <sys/ioctl.h>

int SystemIoPortsNative_Termios2SetSpeed(int fd, int speed)
{
struct termios2 tio2;
if (ioctl(fd, TCGETS2, &tio2) < 0)
{
return -1;
}

tio2.c_cflag &= ~(CBAUD | CBAUDEX | ((CBAUD | CBAUDEX) << IBSHIFT));
tio2.c_cflag |= BOTHER | (BOTHER << IBSHIFT);
tio2.c_ospeed = speed;
tio2.c_ispeed = speed;

if (ioctl(fd, TCSETS2, &tio2) < 0)
{
return -1;
}

return 0;
}


13 changes: 13 additions & 0 deletions src/native/libs/configure.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1157,6 +1157,19 @@ check_symbol_exists(
grp.h
HAVE_GETGRGID_R)

check_c_source_compiles(
"
#include <asm/termbits.h>
#include <sys/ioctl.h>

int main(void)
{
struct termios2 t;
return 0;
}
"
HAVE_TERMIOS2)
Copy link
Member

@krwq krwq Jan 13, 2023

Choose a reason for hiding this comment

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

do we know which rids will not have this defined?

Copy link
Member

@janvorli janvorli Jan 13, 2023

Choose a reason for hiding this comment

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

Actually, we also need to update the eng/native/tryrun.cmake for cross build to get this definition. The live detection in configure.cmake is only for non-cross builds. So we need to get the result of this check on all our cross target platforms (by running the build is running natively there) and update the tryrun.cmake accordingly in this PR.
@krwq that should give an answer to your question then.

Copy link
Member Author

@wfurt wfurt Jan 14, 2023

Choose a reason for hiding this comment

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

According to https://man7.org/linux/man-pages/man2/ioctl_tty.2.html this was added in 2.6.20 Kernel e.g. it should be available in all Linux versions we support. (and is Linux specific) I saw note that it may not work on architectures but I don't know any details. The definition lives in asm-generic/termbits.h so I expect it would compile everywhere.

Copy link
Member

@janvorli janvorli Jan 16, 2023

Choose a reason for hiding this comment

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

Ok, then we can just add it to the eng/native/tryrun.cmake. Please note that in that file, you need to set the HAVE_XXX_EXITCODE, not HAVE_XXX and the exit code is 0 for success.
In other words, you'll need to add

set_cache_value(HAVE_TERMIOS2_EXITCODE 0)

Copy link
Member Author

@wfurt wfurt Jan 16, 2023

Choose a reason for hiding this comment

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

Do we ever use it for OSes other than Linux? (Like FreeBSD ARM64)? I was looking for section with plain LINUX but aside from ALPINE I did not find good fit.

Copy link
Member

@janvorli janvorli Jan 16, 2023

Choose a reason for hiding this comment

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

The section that is used for linux starts here:

runtime/eng/native/tryrun.cmake

Line 74 in 4cf8c07

set_cache_value(FILE_OPS_CHECK_FERROR_OF_PREVIOUS_CALL_EXITCODE 1)

It is used for all cross builds, that means including FreeBSD. Since the section for Linux is shared with FreeBSD, Illumos and Tizen, we need to know the value for those OSes. It seems that it would be as easy as cross building the repo for those three OSes with your change and the tryrun.cmake modified. If it was not supported, the build would fail.

Copy link
Member Author

@wfurt wfurt Jan 16, 2023

Choose a reason for hiding this comment

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

I check FreeBSD headers and there is neither termios2 or TCGETS2. I suspect others would be the same.
Instead of setting negative value for all the other OSes, would it make sense to create new branch for Linux only?
And if do we already have some variable that we can use (CMAKE_SYSTEM_NAME, CLR_CMAKE_TARGET_OS, ..?) or would we need to invent something?

Copy link
Member

@janvorli janvorli Jan 16, 2023

Choose a reason for hiding this comment

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

Since everything else is the same, I'd just make setting this single value conditional based on where it is supported.


configure_file(
${CMAKE_CURRENT_SOURCE_DIR}/Common/pal_config.h.in
${CMAKE_CURRENT_BINARY_DIR}/Common/pal_config.h)