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

Radio settings changed in simulator persist temporarily when they should be reset. #5919

Open
1 task done
philmoz opened this issue Feb 16, 2025 · 22 comments
Open
1 task done
Labels
bug 🪲 Something isn't working color Related generally to color LCD radios simulator

Comments

@philmoz
Copy link
Collaborator

philmoz commented Feb 16, 2025

Is there an existing issue for this problem?

  • I have searched the existing issues

What part of EdgeTX is the focus of this bug?

Simulator

Current Behavior

After setting a custom name on an Axis, Pot or Switch in the simulator the custom name persists after closing and re-opening the simulator.
The custom name is not shown in the companion radio settings, and does not survive closing and re-opening companion.

Expected Behavior

All fields should be reset on closing the simulator window.

Steps To Reproduce

  • Start companion, create a new blank ETX file (no models)
  • Start simulator, a default model is created
  • Set a custom name for an axis, pot or switch in the radio hardware page
  • Close the simulator
  • Run the simulator again - check the axis/pot/switch settings. The names persist.

Version

2.11.0-rc

Transmitter

RadioMaster TX16S / TX16SMK2

Operating System (OS)

No response

OS Version

No response

Anything else?

No response

@philmoz philmoz added bug 🪲 Something isn't working triage Bug report awaiting review / sorting labels Feb 16, 2025
@pfeerick
Copy link
Member

pfeerick commented Feb 16, 2025 via email

@philmoz
Copy link
Collaborator Author

philmoz commented Feb 16, 2025

On MacOs the simulator temporary folder is deleted when the simulator is closed.

@elecpower
Copy link
Collaborator

Ditto on Ubuntu

@elecpower
Copy link
Collaborator

My $2 bet is on a deleteLater and later hasn't come around yet. I haven't checked the code yet so a real guess

@pfeerick
Copy link
Member

pfeerick commented Feb 16, 2025

Spoilsports! 🤣

@elecpower
Copy link
Collaborator

elecpower commented Feb 18, 2025

Tested as per steps above in Ubuntu X9D+, V16, T12MAX, MT12, T20V2 and TX16S. So far only TX16S remembers previous run settings. Even set names before launching simulator and makes no difference.

For all but TX16S the libsim is unloaded on dialog close despite the debug log stating the TX16S libsim was unloaded successfully. The V16 which is also a colour radio unloads so appears not to be colour vs B&W issue.

Checked via cmd 'cat /proc/NNNN/maps | awk '{print $6}' | grep 'libedge' | sort | uniq' where NNNN is the process id of companion from cmd 'top'.

Tested other radios by themselves, before and after TX16S and TX16S is the only one not to unload. TX16S only unloads when the parent Companion executable unloads.

For the TX16S on sim launch the correct radio settings are written to the temp directory but as soon as the libsim radio starts running it overwrites the radio.yml file with the last run data from the libsim.

Open to ideas and wild guesses why TX16S libsim lingers.

@elecpower
Copy link
Collaborator

elecpower commented Feb 18, 2025

Update: in the 1st test the V16 did not have an sd path set and unloaded from memory however a 2nd test with a valid sd path set results in the lib not unloading from memory.

The TX16S remembers with or without sd path being set.

Reloading the radio data via the simulator menu does not clear out the remembered data.

The X9D+ did have a valid sd path set and unloaded.

@philmoz
Copy link
Collaborator Author

philmoz commented Feb 18, 2025

Does not happen in 2.10.

@elecpower
Copy link
Collaborator

I'm going to revert #5778 to see if it makes any difference

@philmoz
Copy link
Collaborator Author

philmoz commented Feb 19, 2025

Looks like it started in commit f28c115 (PR #4407) - the big LVGL refactor.

No idea why.

@elecpower
Copy link
Collaborator

Couldn't start in a small PR could it aaaargh.

@elecpower
Copy link
Collaborator

elecpower commented Feb 19, 2025

When Companion and TX16S simulator running the cmd 'lsof /src/build-debug/native/libedgetx-tx16s-simulator.so' reports in use by Companion and in my case the debugger 'gdb'.
When unloaded the cmd displays the same results.

For X9D+ loaded, same as for TX16S
For X9D+ unloaded, no results displayed as expected.

A diff of Companion process with X9D+ vs TX16S reports just the libsims as differences.

QLibrary(/src/build-debug/native/libedgetx-tx16s-simulator.so)::isLoaded() returns false before the TX16S libsim is loaded and after unloading even thought the OS reports it is loaded.

@elecpower
Copy link
Collaborator

elecpower commented Feb 19, 2025

The issue goes away (well in testing so far) by reverting 5778. So loading and unloading all libsims with 5778 and then load and unload selected libsims works. Why?????

@philmoz
Copy link
Collaborator Author

philmoz commented Feb 19, 2025

I have to go all the way back to just before PR 4407.

Here is a workaround patch that resets the radio and model memory on starting the simulator,

sim_patch.diff.zip

@elecpower
Copy link
Collaborator

Its probably me but the diff appears to be doing lots more than stated objective eg yaml

@philmoz
Copy link
Collaborator Author

philmoz commented Feb 19, 2025

I needed to move the switch and analog name storage into g_eeGeneral to make it easier to clear.

@elecpower
Copy link
Collaborator

elecpower commented Feb 20, 2025

I tested your diff with the deferred libsim loading applied (ie main). The TX16S and V16 libsims behave as expected whether they be new or resident. Great work.

There is still the issue of colour libsims not always unloading. If I start a v16 libsim make changes and close it, it will remain loaded. If I then switch profiles to TX16S and start it, make changes and close, it gets unloaded and the v16 remains loaded.

So far b&w libsims always get unloaded no matter the order.

@philmoz
Copy link
Collaborator Author

philmoz commented Feb 20, 2025

Yeah, I don't know why this is happening - maybe something in LVGL needs shutting down.
I tried deleting all the styles and calling lv_deinit() on shutdown; but it still did not unload, and crashed when restarting the simulator.

@elecpower
Copy link
Collaborator

In my trolling the web for a clue, there are cases of threads pointing to the parent which increments the in use counter stopping the unload if the tread does not clean up before termination. And many more ponderings...

The inconsistency in our case is baffling.

@elecpower
Copy link
Collaborator

elecpower commented Feb 20, 2025

Since the workaround is your work I leave it for you to create the PR as you deserve the credit. Thank you.

I'll keep the issue on my todo list as it could even be a Qt bug. I've started work on upgrading to Qt 6.8 so I'll retest once I have a working environment (it is not pretty at the moment)

@elecpower elecpower added simulator color Related generally to color LCD radios and removed triage Bug report awaiting review / sorting labels Feb 20, 2025
@raphaelcoeffic
Copy link
Member

I needed to move the switch and analog name storage into g_eeGeneral to make it easier to clear.

How is it normally cleared? Isn't it enough to just cancel the temporary files? The current fix moving things back to g_eeGeneral really looks like a way to hide another bug rather than a proper fix, IMHO.

@philmoz
Copy link
Collaborator Author

philmoz commented Feb 26, 2025

As noted in the workaround PR the underlying cause in unknown. If you have a better fix fell free to update the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working color Related generally to color LCD radios simulator
Projects
None yet
Development

No branches or pull requests

4 participants