Skip to content
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

♻️ refactor by unsafe code #3

Merged
merged 4 commits into from
May 17, 2023

Conversation

frg2089
Copy link
Contributor

@frg2089 frg2089 commented May 14, 2023

This should provide a performance boost.

I added code to kill the pico_et_ft_bt_bridge.exe process and it may now be possible to run the module without removing pico_et_ft_bt_bridge.exe.
5bbb230#diff-6e070d01f5c9e071f65b958859432bceaa872dfc097100051e5414f9641afcc9R56-R80

And I added code to check the Streaming Assistant process, so that the module will not time out when the process exists, but will simply fail when the process does not exist.
5bbb230#diff-6e070d01f5c9e071f65b958859432bceaa872dfc097100051e5414f9641afcc9R34-R38

Signed-off-by: 舰队的偶像-岛风酱! <frg2089@outlook.com>
Signed-off-by: 舰队的偶像-岛风酱! <frg2089@outlook.com>
Signed-off-by: 舰队的偶像-岛风酱! <frg2089@outlook.com>
@regzo2
Copy link
Owner

regzo2 commented May 14, 2023

Thanks! I will test it out and merge when I get back home

@frg2089
Copy link
Contributor Author

frg2089 commented May 15, 2023

In my testing, I can now switch between Streaming Assistant and VRCFT data streams at any time.

When running VRCFT, the module will kill the pico_et_ft_bt_bridge.exe process and listen to the port before it does, at which point VRChat will receive data from VRCFT.
After closing off VRCFT, pico_et_ft_bt_bridge.exe takes over the data again, and VRChat receives data from Streaming Assistant at that time.

@frg2089
Copy link
Contributor Author

frg2089 commented May 16, 2023

There seems to be a memory leak somewhere, I'll close this PR first.

@frg2089 frg2089 closed this May 16, 2023
Signed-off-by: 舰队的偶像-岛风酱! <frg2089@outlook.com>
@frg2089
Copy link
Contributor Author

frg2089 commented May 16, 2023

I didn’t find any memory leaks, but I fixed a busy wait.

@frg2089 frg2089 reopened this May 16, 2023
@regzo2
Copy link
Owner

regzo2 commented May 17, 2023

Seems to be very stable. I have a few people that have the Pico4Pro/Enterprise testing the changes out. All the changes seem to work pretty well so I'll go ahead and merge it! I am going to make a couple of changes regarding threading of the Update function (since it currently eats up a bunch of CPU cycles) but otherwise seems good to go.

@regzo2 regzo2 merged commit f20ac8f into regzo2:vrcfacetracking-module May 17, 2023
@regzo2
Copy link
Owner

regzo2 commented May 17, 2023

Looks like you already did the threading change! need to add a timeout so the Update thread doesn't get halted indefinitely on teardown (edge case). I will have that on the 1.2 release

lonelyicer referenced this pull request in lonelyicer/PicoConnectFTUDP Feb 8, 2024
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