Skip to content

Fixed error if API version not received. #203

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

Merged
merged 3 commits into from
Apr 9, 2019

Conversation

mikeller
Copy link
Member

No description provided.

hydra
hydra previously approved these changes Apr 2, 2019
Copy link

@hydra hydra left a comment

Choose a reason for hiding this comment

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

fix looks ok, but as noted via slack there is the possibility for an infinite loop which is currently unhandled.

@mikeller
Copy link
Member Author

mikeller commented Apr 3, 2019

@hydra: I found a simple way to fix the infinite loop issue - now the behaviour is that if there are no pages with a suitable version found, the script cycles through all of the unsuitable pages as if there was no check. I don't think there is a way for the script to self-terminate - the ui routine will just be called over and over again.

@mikeller mikeller requested a review from codecae April 4, 2019 09:28
@codecae
Copy link
Member

codecae commented Apr 4, 2019

Still looking at this... since run_ui is being used by the telemetry script as the run script, returning non-zero will cause the telem script to exit gracefully.

Will keep digging..

while Page == nil do
Page = assert(loadScript(radio.templateHome .. PageFiles[currentPage]))()
if Page.requiredVersion and Page.requiredVersion > apiVersion then
if Page.requiredVersion and apiVersion and Page.requiredVersion > apiVersion and currentPage ~= nextPage then
Copy link
Member

@codecae codecae Apr 5, 2019

Choose a reason for hiding this comment

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

apiVersion will always evaluate to true if it is not nil.

> x = 0
> if x then print(1) end
1

Given that apiVersion is initialized as 0 and is never nil, the boolean check for this variable will always be true

Perhaps nesting currentPage ~= nextPage within this if statement would permit a way to gracefully exit when they are equivalent.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have added exiting if no valid pages are found.

@mikeller
Copy link
Member Author

mikeller commented Apr 9, 2019

Anybody in for a review?

@mikeller mikeller merged commit f022976 into betaflight:master Apr 9, 2019
@mikeller mikeller deleted the fix_nil_error branch April 9, 2019 10:53
@mikeller mikeller mentioned this pull request Apr 14, 2019
@hsuq510
Copy link

hsuq510 commented Apr 15, 2019

didnt work on X9D, apiversion still not received. Within data initialized in ui works.

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.

5 participants