Skip to content

Fix UDP terminal handling and X11/libevent cleanup #105

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
tipoman9 merged 3 commits into from
Aug 16, 2026
Merged

Fix UDP terminal handling and X11/libevent cleanup #105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e938ffc
Fix repeated X11 event registration
etiaro Jul 24, 2026
0acfa59
Fix libevent shutdown ownership
etiaro Jul 24, 2026
9e2f9eb
Preserve terminal settings in UDP mode
etiaro Jul 24, 2026
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
20 changes: 14 additions & 6 deletions msposd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ extern char* recording_dir;
// libevent base main loop
struct event_base *base = NULL;

int serial_fd = 0;
int serial_fd = -1;
int in_sock = 0;
int MSPUDPPort = 0;
int MSP_PollRate = 20;
Expand Down Expand Up @@ -1182,7 +1182,7 @@ static void poll_msp(evutil_socket_t sock, short event, void *arg) {

static int handle_data(const char *port_name, int baudrate, const char *out_addr) {
struct event *sig_int = NULL, *in_ev = NULL, *temp_tmr = NULL, *msp_tmr = NULL;
struct event *sig_term;
struct event *sig_term = NULL;
int ret = EXIT_SUCCESS;

// Read from UDP
Expand Down Expand Up @@ -1217,6 +1217,10 @@ static int handle_data(const char *port_name, int baudrate, const char *out_addr
printf("Listening UART on %s...\n", port_name);
}

// In UDP mode there is no UART; do not apply raw termios settings to stdin.
if (serial_fd < 0)
goto uart_configured;

struct termios options;
tcgetattr(serial_fd, &options);
cfsetspeed(&options, speed_by_value(baudrate));
Expand Down Expand Up @@ -1253,6 +1257,8 @@ static int handle_data(const char *port_name, int baudrate, const char *out_addr
msp_set_vtx_config(serial_fd);
}

uart_configured:

if (strlen(out_addr) > 1) {
out_sock = socket(AF_INET, SOCK_DGRAM, 0);

Expand Down Expand Up @@ -1280,7 +1286,7 @@ static int handle_data(const char *port_name, int baudrate, const char *out_addr
// Test inject a simple packet to test malvink communication Camera to Ground
signal(SIGUSR1, sendtestmsg);

if (serial_fd > 0 && !enable_simple_uart) { // if UART opened and we need to read it via events
if (serial_fd >= 0 && !enable_simple_uart) { // if UART opened and we need to read it via events
serial_bev = bufferevent_socket_new(base, serial_fd, 0);

// Trigger the read callback only whenever there is at least 16 bytes of data in the buffer.
Expand Down Expand Up @@ -1355,7 +1361,7 @@ static int handle_data(const char *port_name, int baudrate, const char *out_addr

// MSP_PollRate
if (ParseMSP && msp_tmr == NULL &&
serial_fd > 0) { // Only if we are on Cam, on ground no need to poll
serial_fd >= 0) { // Only if we are on Cam, on ground no need to poll
msp_tmr = event_new(base, -1, EV_PERSIST, poll_msp, &serial_fd);
// Set poll interval to 50 milliseconds if pollrate is 20
struct timeval interval = {
Expand Down Expand Up @@ -1390,14 +1396,16 @@ static int handle_data(const char *port_name, int baudrate, const char *out_addr

if (sig_int)
event_free(sig_int);
if (sig_term)
event_free(sig_term);

CloseMSP();

if (base)
event_base_free(base);

libevent_global_shutdown();

CloseMSP();

return ret;
}

Expand Down
69 changes: 55 additions & 14 deletions osd/util/Render_gs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,9 +516,10 @@ void FlushDrawing() {
if (x11_event == NULL && base != NULL) {
// Attach X11 display's file descriptor to the existing msposd
// event_base
struct event *x11_event =
x11_event =
event_new(base, ConnectionNumber(display), EV_READ | EV_PERSIST, event_callback, NULL);
event_add(x11_event, NULL);
if (x11_event)
event_add(x11_event, NULL);

XGrabKey(display, XKeysymToKeycode(display, XK_Up), Mod1Mask, RootWindow, True,
Comment on lines +521 to 524

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Aug 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Informational

1. Ungated xgrabkey on failure 🐞 Bug ☼ Reliability

In FlushDrawing(), XGrabKey() executes even if event_new() fails, leaving x11_event NULL; since
FlushDrawing() is called repeatedly, this can cause repeated hotkey-grab attempts every frame
without ever registering the X11 fd with libevent. This can create noisy X11 errors/unnecessary work
and still leaves keyboard event handling disabled.
Agent Prompt 
### Issue description
`FlushDrawing()` unconditionally calls `XGrabKey()` after attempting `event_new()`. If `event_new()` (or `event_add()`) fails, `x11_event` remains `NULL`, and the next `FlushDrawing()` call will retry and repeatedly attempt key grabs every frame.

### Issue Context
This code runs in the render loop; `FlushDrawing()` is invoked repeatedly during normal operation.

### Fix Focus Areas
- osd/util/Render_gs.c[516-530]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

GrabModeAsync,
Expand Down Expand Up @@ -565,15 +566,54 @@ void FlushDrawing() {

void Close() {
// Clean up resources
cairo_destroy(cr);
cairo_destroy(cr_back);
cairo_surface_destroy(image_surface);
cairo_surface_destroy(surface);
cairo_surface_destroy(surface_back);
if (image_surface) {
cairo_surface_destroy(image_surface);
image_surface = NULL;
}
#if defined(_x86)
if (x11_event) {
event_del(x11_event);
event_free(x11_event);
x11_event = NULL;
}

if (!shm_image) {
if (cr) {
cairo_destroy(cr);
cr = NULL;
}
if (surface) {
cairo_surface_destroy(surface);
surface = NULL;
}
} else {
cr = NULL;
surface = NULL;
}
#else
if (cr) {
cairo_destroy(cr);
cr = NULL;
}
if (surface) {
cairo_surface_destroy(surface);
surface = NULL;
}
#endif
if (cr_back) {
cairo_destroy(cr_back);
cr_back = NULL;
}
if (surface_back) {
cairo_surface_destroy(surface_back);
surface_back = NULL;
}

#if defined(_x86)
// Clean up XShm triple-buffer resources
if (shm_image) {
XShmDetach(display, &shm_seg);
if (display)
XShmDetach(display, &shm_seg);
XDestroyImage(shm_image);
shm_image = NULL;
}
Expand Down Expand Up @@ -604,12 +644,13 @@ void Close() {
shm_sysv_id = -1;
}

XDestroyWindow(display, window);
XCloseDisplay(display);

// Cleanup
event_free(x11_event);
event_base_free(base);
if (display) {
if (window)
XDestroyWindow(display, window);
XCloseDisplay(display);
display = NULL;
window = 0;
}
#endif
}

Expand Down
Loading