Skip to content

Fix chat module bugs #1561

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 21 additions & 3 deletions MAVProxy/modules/mavproxy_chat/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def __init__(self, mpstate):
super(chat, self).__init__(mpstate, "chat", "OpenAI chat support")

# register module and commands
self.add_command('chat', self.cmd_chat, "chat module", ["show"])
self.add_command('chat', self.cmd_chat, "chat module", ["hide", "show"])

# keep reference to mpstate
self.mpstate = mpstate
Expand All @@ -42,29 +42,47 @@ def __init__(self, mpstate):

# create chat window (should be called from a new thread)
def create_chat_window(self):
print("Trying to create chat window")
Copy link
Contributor

Choose a reason for hiding this comment

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

we should remove this debug output

if mp_util.has_wxpython:
# create chat window
# create chat window, and return the chat_window object created
self.chat_window = chat_window.chat_window(self.mpstate, self.wait_for_command_ack)
# Call main loop of chat window
self.chat_window.start()
else:
print("chat: wx support required")

# show help on command line options
def usage(self):
return "Usage: chat <show>"
return "Usage: chat <hide|show>"

# control behaviour of the module
def cmd_chat(self, args):
if len(args) == 0:
print(self.usage())
elif args[0] == "show":
self.show()
elif args[0] == "hide":
self.hide()
else:
print(self.usage())

# show chat input window
def show(self):
self.chat_window.show()

def hide(self):
self.chat_window.hide()

def close(self):
self.chat_window.close()

# unload function override for module interface
def unload(self):
Copy link
Contributor

@rmackay9 rmackay9 May 1, 2025

Choose a reason for hiding this comment

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

I've tested "module unload chat" and it doesn't seem to work I'm afraid. Please tell me if I'm doing something wrong. I get the message shown below. Perhaps it's because I'm using WSL2?

STABILIZE> Unloaded module chat
Trying to create chat window
Reloaded module chat

(mavproxy.py:7420): Gtk-CRITICAL **: 21:57:40.028: gtk_box_gadget_distribute: assertion 'size >= 0' failed in GtkScrollbar
[xcb] Unknown request in queue while dequeuing
[xcb] Most likely this is a multi-threaded client and XInitThreads has not been called
[xcb] Aborting, sorry about that.
python3: ../../src/xcb_io.c:175: dequeue_pending_request: Assertion `!xcb_xlib_unknown_req_in_deq' failed.
SIM_VEHICLE: MAVProxy exited
SIM_VEHICLE: Killing tasks

Copy link
Author

Choose a reason for hiding this comment

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

I have only tested on Windows so far. I will look into it.

Copy link
Author

Choose a reason for hiding this comment

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

I looked into why it was not working in Linux (I tried in Ubuntu to recreate the error). It seems wxPython is built on top of GTK in Ubuntu, which is much stricter than the Windows version. It is not allowing reinitializing app or rerunning mainLoop in the same process. I tried different approaches to getting it working in the same process, but keep getting seg fault.
I think it would be better to launch a different process for the app using multiprocessing. What are your thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

Another approach would be to:

  • Hide the chat window and clear the data (unloading).
  • Show the window along with new assistant thread (reloading).

I think this might be better, since there is no need to rework the implementation for handling mavlink acknowledgement.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @SriramS-77,

Either way sounds good to me. By the way, do you have an OpenAPI key? I just checked the list of ArduPilot sponsored API keys and I don't see you there. If you'd like one, please just PM me on ArduPilot's discord server, I'm "rmackay9" over there too.

# Close the chat window
self.close()
# Call the unload function of super class, to include its functionality
super(chat, self).unload()

# handle mavlink packet
def mavlink_packet(self, m):
if m.get_type() == 'COMMAND_ACK':
Expand Down
3 changes: 2 additions & 1 deletion MAVProxy/modules/mavproxy_chat/chat_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,13 @@ def __init__(self, mpstate, wait_for_command_ack_fn):
# show frame
self.frame.Show()

def start(self):
# chat window loop (this does not return until the window is closed)
self.app.MainLoop()

# show the chat window
def show(self):
wx.CallAfter(self.frame.Show())
wx.CallAfter(self.frame.Show)

# hide the chat window
def hide(self):
Expand Down