Skip to content

Fix chat module bugs#1561

Open
SriramS-77 wants to merge 2 commits intoArduPilot:masterfrom
SriramS-77:master
Open

Fix chat module bugs#1561
SriramS-77 wants to merge 2 commits intoArduPilot:masterfrom
SriramS-77:master

Conversation

@SriramS-77
Copy link
Copy Markdown

Fixes:

Added "chat hide" feature, to interact with existing "chat show" feature.

Demo Video:

Git_PR_Demo.MP4

@rmackay9
Copy link
Copy Markdown
Contributor

rmackay9 commented May 1, 2025

Hi @SriramS-77,

Thanks very much for this!

Two small administrative things, could you add a "Chat: " prefix to the commit title? To be clear I mean the commit title not the PR title. Also if possible could you rebase on master to remove the "Merge branch 'master' into master" commit?

I'll give this a quick test shortly but it looks good, thanks!


# create chat window (should be called from a new thread)
def create_chat_window(self):
print("Trying to create chat window")
Copy link
Copy Markdown
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

Comment thread MAVProxy/modules/mavproxy_chat/chat_window.py
self.chat_window.close()

# unload function override for module interface
def unload(self):
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

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