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

Bit at Index 64 is set to 0 #831

Open
shihaocao opened this issue Oct 15, 2021 · 18 comments
Open

Bit at Index 64 is set to 0 #831

shihaocao opened this issue Oct 15, 2021 · 18 comments

Comments

@shihaocao
Copy link
Collaborator

image

@shihaocao
Copy link
Collaborator Author

git checkout gpstime-test

ws a time.gps, notice that after telem dump + parsetelem that all odd numbered wn don't parse correctly

@shihaocao
Copy link
Collaborator Author

image

@shihaocao
Copy link
Collaborator Author

image

@shihaocao
Copy link
Collaborator Author

python -m ptest runsim -c ptest/configs/hootl.json -t SingleSatDetumbleCase --clean

@shihaocao
Copy link
Collaborator Author

Moving time.gps to the front of the flow fixes it?

@shihaocao
Copy link
Collaborator Author

nevermind, there's some code that's setting the 2nd bit from the front of the flow to 0. when time.gps is at the front of a flow, 2045->2045, 2046->2044, 2047->2045, 2048->2048

@shihaocao shihaocao changed the title GPS Time Deserializer Not Working 2nd Flow Bit being set to 0 Oct 16, 2021
@shihaocao shihaocao changed the title 2nd Flow Bit being set to 0 3nd Flow Bit being set to 0 Oct 16, 2021
@shihaocao
Copy link
Collaborator Author

Planned course of action is to move attitude_estimator.ignore_sun_vectors to the 3rd bit in front of GPS time. that means that field will always be false, but at least the other fields will be intact we thnk.

We have no idea if any of the other flows below flow 2 are affected.

@shihaocao
Copy link
Collaborator Author

image
with the 2nd flow and 3rd flow swapped positions, we note that velocity also happens to be negative when it should be positive...

This output was generated with the new FlowInpsect ptest case

@shihaocao
Copy link
Collaborator Author

As far as we can tell, this cursed bit effect also goes away when we changed the boot count serializer with #828

@shihaocao
Copy link
Collaborator Author

shihaocao commented Oct 27, 2021

Actually, it moves it into the 26th bit of the second flow. Which is the in the time of week....

@afoarce
Copy link
Collaborator

afoarce commented Oct 31, 2021

image
In printing snapshots over time, it appears that the 'cursed' bit flips when the packet splits based on testing on branch cursed-testing

@afoarce
Copy link
Collaborator

afoarce commented Oct 31, 2021

I believe it is this

packet_start = bit_array::modify_bit(packet_start, 7, 0);
that clears the bit due to a miscalculation on
char& packet_start = snapshot_ptr[(downlink_frame_offset / 70)];
. When splitting packets, the downlink_frame_offset/70 gives 8 (bytes) and so the 64th bit is cleared instead of the first bit after 70 bytes. The packet header is still added successfully because snapshot is initialized to all 0s and the offset is incremented, so the 561st bit remains 0. Testing with the modify_bit line commented out shows no cleared bit.

Unfortunately, this means the combination of changing the serializer size for bootcount and adding the cursed bit in #838 moved the issue but did not mitigate it. Further testing needed to quantify the effects on the accuracy of time.gps.

@shihaocao @tanishqaggarwal @kylekrol

@shihaocao
Copy link
Collaborator Author

wow that's crazy

@shihaocao shihaocao changed the title 3nd Flow Bit being set to 0 Bit at Index 64 is set to 0 Nov 6, 2021
@shihaocao
Copy link
Collaborator Author

The bit that is being set is now in pan.cycle_no, therefore we are fine.

@shihaocao
Copy link
Collaborator Author

image

@shihaocao
Copy link
Collaborator Author

image
In the second half of the packet, the rwa_torque_cmd bit will be set to zero as it takes up 53 - 65

@knk38
Copy link
Member

knk38 commented Nov 6, 2021

Resolution is to turn off cursed flows, and only toggle on these flows:

  • 14, 15
  • 17, 18, 19
  • 25, 26

@shihaocao
Copy link
Collaborator Author

Or we change the downlink parser code to go as follows:
if a packet claims it is the first packet in a new frame, parse it immediately, check its flow_id values in the expected positions if they don't comprise the flow_id values we expect in the first packet of a new frame, conclude that it must be a packet a part of the previous frame.

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

No branches or pull requests

3 participants