-
Notifications
You must be signed in to change notification settings - Fork 0
Bug with how I reimplemented Arithmatic #20
Merged
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
There is a problem that may be cause by these changes, that being not closing the handle, since the loop will run forever, I would have to close the loop on some sort of exit condition. I am not sure if the handle closes if the process is killed, if so, then I guess it should be fine.
This reverts commit 5a9cb08.
This reverts commit e2ea108.
I have no idea why exactly but the build errors only got fixed when I changed the order of the includes in AHRSInputIOProcessor.hpp.
I added some comments explaining how I am capturing the bytes of the Euler Angles from the message in the buffer. Quick version here, I first capute a 2 times the size of the message into a buffer, I then search for the begining of the message, when I find it, I then jump to the index where the Angles are stored. I then add each group of 8 bytes for each angle backwords into arrays to then be converted. I add them in backwords since the AHRS is big endian and the computer we are converting them to doubles on is little endian. After each angle's 8 bytes are storred in an array, I then cast them into doubles, where they can then be sent. Hopefully that makes sense, feel free to ask me any questions about it.
I fixed the math for getting the bytes for the Angles to make a bit easier to understand. I also clarfied my comments a bit to try and make them clearer with the explanation.
I further clarified my comments and also renamed index to dataStart to make it more clear what it was.
Changed "index" to "dataStart" in diagram to make it more clear.
Changed the for loop to loop backwords itself so that the equations for actually getting the bytes only use +'s. They are still basically the same equations, but hopefully they are a little more intuitive...
This reverts commit 13c66ec.
This reverts commit 9988615.
This reverts commit f50c2f1.
This reverts commit 596b9af.
This reverts commit 361589d.
When I changed the arithmetic around it messed up the indexing of the byte arrays. The reason i went up(forward) was so that I could put it in the array, not to do some funky math. If we don't revert these changes the byte order will simply be wrong.
Hopefully it makes sense why this needs to be reverted. |
Sign in
to join this conversation on GitHub.
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 way I re implemented the arithmetic to "make it more intuitive" broke it. The reason I had it count up was because the arrays that would actually be holding the 8 bytes of data was being populated in order. By changing the math to go the other way, I would either have to change around the equations again, or add a funky equation into the indexing of the byte arrays. This pull request reverts the arithmetic changes.