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

Fixed GUI crashes on RaspberryPi. #221

Closed
wants to merge 1 commit into from

Conversation

GewoonGijs
Copy link
Contributor

Hi Wouter,

I tested my fixes today with my Tacx and succesfully made my first trip on Zwift.

The mx.CallAfters() around the update of the gauges might not be absolutely necessary but for the responsiveness of the GUI buttons I think it is a huge improvement. All the other locations have the risk of being called from another than the GUI thread.

@GewoonGijs
Copy link
Contributor Author

Fixed #216

@WouterJD
Copy link
Owner

Gijs, welkom!
Welcome in the FortiusANT community and thanks for studying the subject.

Before merging the request, I would like to understand what issue is being resolved.
If I understand well what I did, the only wx-calls are done from the GUI thread.

I will study your code suggestions asap; and do some other question-answering first.

@GewoonGijs
Copy link
Contributor Author

Hi Wouter,

The problem arises because a separate Thread is started after the RunOff or Start button is pressed.

In FortiusANTGui.py this results in calling callRunOff() in a "non-gui" thread and not the main wx thread (line 1193) the same issue is with callTacx2Dongle() (line 1233).

This results in calling setValues() and setMessages() in GuiMessage2Main() from the "non-gui Thread", and in these functions all the updates to the GUI are executed.

It took me quite a while to find this little nasty bug.

@WouterJD
Copy link
Owner

WouterJD commented Jan 25, 2021

Looking at your implementation I would prefer below code

    def SetValues(self, fSpeed, iRevs, iPower, iTargetMode, iTargetPower, fTargetGrade, \
                        iTacx, iHeartRate, \
                        iCrancksetIndex, iCassetteIndex, fReduction):
        wx.CallAfter(
                    self.SetValuesGUI, \
                    fSpeed, iRevs, iPower, iTargetMode, iTargetPower, fTargetGrade, \
                    iTacx, iHeartRate, \
                    iCrancksetIndex, iCassetteIndex, fReduction)

    def SetValuesGUI(self, fSpeed, iRevs, iPower, iTargetMode, iTargetPower, fTargetGrade, \
                        iTacx, iHeartRate, \
                        iCrancksetIndex, iCassetteIndex, fReduction):

because it allows "anybody" to call SetValues() and solve the problem once.

I was not aware that wx had to be called from the main thread only.
Therefore CallAfter is new to me (thanks for teaching this) so I'm interested in your view.

PS: Same construct to be done for SetMessages() and PedalStrokeAnalysis()

@WouterJD
Copy link
Owner

WouterJD commented Jan 25, 2021

I hope this image helps.

image

@WouterJD
Copy link
Owner

WouterJD commented Jan 25, 2021

Please check https://github.com/WouterJD/FortiusANT/blob/%23216-GUI-crash-on-raspberry/pythoncode/FortiusAntGui.py

I have done a quick test, verifying functionality.
Please confirm this is good for Raspberyy as well.

Similar issues are resolved in [Stop] button after [Runoff] and [Start]

@GewoonGijs
Copy link
Contributor Author

@wouter, I totally agree with you, the solution you proposed is cleaner and more robust. I just tested it in simulation mode, and it runs just as smoothly as the fixes I tried before. And that makes sense.

Thanks alot, I will close the PR.

@GewoonGijs GewoonGijs closed this Jan 25, 2021
@GewoonGijs GewoonGijs deleted the fix-Gui-crash branch January 25, 2021 20:12
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