Define UPS_States values as pow of 2 to fix state change detection #71
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Sorry for taking so long to get to this; usual real life activities so projects had to be put on hold. I could've sworn enum values were auto-generated in powers of two when using the FlagsAttribute, but sure enough, they need to be manually defined. Thank you so much for this, I'm just trying to get my VM with upsd up so I can figure out how I even missed this and then merge your changes. Thank you for contributing! |
|
Hey @gbakeman. You have nothing to be sorry about. It's open source, everyone has different capacity for this kind of work. I'd actually say it was a fairly decent response time considering that you're just a single maintainer here and have other job. Thanks for finding time to take a look at it and merging! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The existing state detection logic relies on bitwise operations to compare old and new state and to extract the changed state flags. However it's fundamentally broken due to UPS_States enum not using the power of 2 values. Bitwise logic produces invalid states change set because of that.
I discovered it while I was trying to make this client follow FSD signal from the server. But it never detects the change for FSD state. The only case when it starts the shutdown procedure is if you connect to the server which is already in FSD state.