add second ODS capture + delayed first capture

[dhacks541]

ODS Capture Changes

Summary of changes

Motivation of this PR is to increase the likelihood of a successful capture of the lightsail after deployment via the ODS. This involves tweaking the timing of the first photo, as well as taking a supplemental backup photo shortly after the first, in the event that the timing/spin of the CubeSat does not produce a good first image.

• CameraControlTask.cpp modified to automatically trigger a second photo capture during deployment, ~4.6 seconds after the first. This capture occurs after the first photo is written to the SD card, and involves resuming and recapturing the camera frame before ultimately powering off the camera and SD.
• Initial photo capture sequence modified to pause for two control cycles (200 ms) after open door toggles sfr::camera::take_photo before capturing photo. This is accomplished by continuing to increment the sfr::camera::start_progress field (used to monitor camera initialization progress) from 6 to 8 before calling adaCam.takePicture(). Consider renaming sfr field if this variable reuse justifies changes to sfr references and GS, but would like to avoid this, since the SFR field is still being used for its original purpose of incrementally triggering camera operations. This change was precipitated by testing on the flight unit in February 2025 that revealed Spring 2022 testing (which had indicated no additional delay was necessary between button depress and photo capture for exposure or framing purposes) did not match results on the flight unit. Potentially due to hardware differences in button responsiveness, camera exposure adjustment speed, or speed of light sail deployment with the flight sail vs dummy.
• Additionally, sfr::camera::start_progress reset to 0 after ODS shutdown. In current flight software, if operators attempted to command a supplemental camera capture at some point after deployment, camera initialization would not be successful without the progress counter being reset.
• Length of the camera report changed from 70 bytes (64 bytes photo data) to 86 bytes (80 bytes photo data) in order to reduce total number of camera fragments for 2 photos below 100. Testing revealed that reading more than 100 consecutive camera fragments from VC-0706 unit resulted in corrupted fragments, due to presumed hardware limitations.

Testing

• Extensive flatsat testing in F24 semester confirmed the functionality of double photo capture change, including GS decoding and no significant negative power budget results involved with transmitting two photos. See FA24 semester report for documentation. This testing revealed that resumeVideo() must be called before calling takePicture() again in order to avoid rereading the same image data as a "second photo" when writing to the SD card.
• Subsequent testing in January 2025 indicated that issues downlinking corrupted/incomplete second images were due to a bug when attempting to read more than 100 consecutive camera fragments from the camera module. Indirect references to this limitation were found in online documentation for the VC-0706 module, although the exact causation was not determined. As a workaround, the size of each camera report was increased by ~20% in order to reduce the total number of camera fragments written, after discussions with Josh indicated that the 70-byte report size specified in flight software was entirely arbitrary (although smaller values result in a shorter transmission time which increases the likelihood of successful transmission), and could (from a Rockblock perspective) be doubled or tripled if necessary.
• Testing in February and March 2025 confirmed that this change to ODS report size has no negative impacts on power budget, requires no changes to the ground station for decoding, and requires no changes to the flight software besides modification of the constants::camera::content_length value. Validation of both the report size change and addition of a second photo was accomplished with the following configurations: flatsat with Rockblock simulator, flatsat with ground Rockblock/ground station in the loop, EDU with rockblock simulator, flight unit with rockblock simulator. This testing occurred largely after nominal testing on the flight unit with GS in the loop, as the acceptance-tested CubeSat can no longer be used for deployment tests. Formal test summaries haven't been compiled yet due to timeline constraints, but can be produced if concerns exist about any of this testing.

SFR Changes

• No sfr changes, but constants::camera::content_length changed from 64 to 80 to increase the size of the camera report by 16 bytes
• (Edit 3/11): delay counter changed from hardcoded value [8 = 6(prexisting value) +2 (control cycles)] to reference a camera::delay_count SFR field, to enable flexibility in sail deployment timing should additional testing reveal an alternate value should be used

[dhacks541]

btw, any idea how to get rid of these formatting fails? there seems to be conflict between clang-format and the pipeline format task

[dhacks541]

Was thinking about this some more, and wondering if the delay threshold that triggers photo capture should just be a sfr value, so we can change it by uplink if we are able to refine our predictions for sail deployment mechanics/timing before mission operations? If so, how much testing would you guys be comfortable with to verify that the extra byte in the normal report doesn't cause any issues (or do we just leave it out)?

[dhacks541]

Was thinking about this some more, and wondering if the delay threshold that triggers photo capture should just be a sfr value, so we can change it by uplink if we are able to refine our predictions for sail deployment mechanics/timing before mission operations? If so, how much testing would you guys be comfortable with to verify that the extra byte in the normal report doesn't cause any issues (or do we just leave it out)?

Pushed a new commit, made the executive decision to include in SFR. Think the benefit to the mission justifies not having a hardcoded reference that we can't change, while it doesn't justify modifying the normal report to include this value (that while useful, does not change operations if can't verify its value). But curious for your input. This has been tested with GS in the loop on a flatsat nominal test, including overriding the default value (two control cyles).

One question I have, is it a problem to have a uint8_t SFR field at the end of a bunch of uint32_t SFRFields (for reasons besides formatting). Didn't want to have to change all the uint32 opcodes..

[ljgreenhill]

Was thinking about this some more, and wondering if the delay threshold that triggers photo capture should just be a sfr value, so we can change it by uplink if we are able to refine our predictions for sail deployment mechanics/timing before mission operations? If so, how much testing would you guys be comfortable with to verify that the extra byte in the normal report doesn't cause any issues (or do we just leave it out)?

Pushed a new commit, made the executive decision to include in SFR. Think the benefit to the mission justifies not having a hardcoded reference that we can't change, while it doesn't justify modifying the normal report to include this value (that while useful, does not change operations if can't verify its value). But curious for your input. This has been tested with GS in the loop on a flatsat nominal test, including overriding the default value (two control cyles).

One question I have, is it a problem to have a uint8_t SFR field at the end of a bunch of uint32_t SFRFields (for reasons besides formatting). Didn't want to have to change all the uint32 opcodes..

I think if we had this it should be included in the normal report. And I agree- totally fine to just tack it onto the end instead of changing the opcodes.

[dhacks541]

Was thinking about this some more, and wondering if the delay threshold that triggers photo capture should just be a sfr value, so we can change it by uplink if we are able to refine our predictions for sail deployment mechanics/timing before mission operations? If so, how much testing would you guys be comfortable with to verify that the extra byte in the normal report doesn't cause any issues (or do we just leave it out)?

Pushed a new commit, made the executive decision to include in SFR. Think the benefit to the mission justifies not having a hardcoded reference that we can't change, while it doesn't justify modifying the normal report to include this value (that while useful, does not change operations if can't verify its value). But curious for your input. This has been tested with GS in the loop on a flatsat nominal test, including overriding the default value (two control cyles).
One question I have, is it a problem to have a uint8_t SFR field at the end of a bunch of uint32_t SFRFields (for reasons besides formatting). Didn't want to have to change all the uint32 opcodes..

I think if we had this it should be included in the normal report. And I agree- totally fine to just tack it onto the end instead of changing the opcodes.

I did initially test with it included in the normal report, as I thought from my look at it that doing so would just involve adding a serialize(0x2019) to the vector (along with updating the sfr field's initilization to have mapping bounds). However this resulted in the normal report becoming largely corrupted. Is there a simple fix here? Any testing would also have to be done today, so I am ok reverting this to a hardcoded value if you don't think this can be added to the sfr without inclusion in the NR.

[Duncan-McD]

btw, any idea how to get rid of these formatting fails? there seems to be conflict between clang-format and the pipeline format task

What version of clang-format do you have locally? The pipeline seems to be using version 13 with default LLVM formatting - guessing your local version is configured different

[Duncan-McD]

Was thinking about this some more, and wondering if the delay threshold that triggers photo capture should just be a sfr value, so we can change it by uplink if we are able to refine our predictions for sail deployment mechanics/timing before mission operations? If so, how much testing would you guys be comfortable with to verify that the extra byte in the normal report doesn't cause any issues (or do we just leave it out)?

Sorry that I'm late to the discussion here - personally, I don't think its a hard requirement to add the field to the NR if we make it an SFR field, but it would be highly ideal - Not having it in NR does make it difficult to gauge what the current value is set to (The only verification that your change was actually accepted correctly would be through the command history telem, but if we sent multiple commands to the same field, we wouldn't know which one stuck)

Also of note, we would need to update any SFR related eeprom constants - Should just be constants::num_sfr_fields

looking back at Eric's report, he calculated mission time metrics off of 97 SFR fields, so adding one more should not be a problem (same amount of eeprom shards regardless)

[Duncan-McD]

The change makes sense to me - I'd be okay adding the SFR as long as we update eeprom constant. Would be ideal to add to NR, but I don't know that this is realistic to get figured out and verified before final upload

[Duncan-McD]

Was thinking about this some more, and wondering if the delay threshold that triggers photo capture should just be a sfr value, so we can change it by uplink if we are able to refine our predictions for sail deployment mechanics/timing before mission operations? If so, how much testing would you guys be comfortable with to verify that the extra byte in the normal report doesn't cause any issues (or do we just leave it out)?

Pushed a new commit, made the executive decision to include in SFR. Think the benefit to the mission justifies not having a hardcoded reference that we can't change, while it doesn't justify modifying the normal report to include this value (that while useful, does not change operations if can't verify its value). But curious for your input. This has been tested with GS in the loop on a flatsat nominal test, including overriding the default value (two control cyles).
One question I have, is it a problem to have a uint8_t SFR field at the end of a bunch of uint32_t SFRFields (for reasons besides formatting). Didn't want to have to change all the uint32 opcodes..

I think if we had this it should be included in the normal report. And I agree- totally fine to just tack it onto the end instead of changing the opcodes.

I did initially test with it included in the normal report, as I thought from my look at it that doing so would just involve adding a serialize(0x2019) to the vector (along with updating the sfr field's initilization to have mapping bounds). However this resulted in the normal report becoming largely corrupted. Is there a simple fix here? Any testing would also have to be done today, so I am ok reverting this to a hardcoded value if you don't think this can be added to the sfr without inclusion in the NR.

Looking through the code, I can't seem to see why this would have caused it to be corrupt - did you place the new field at the complete end? or between the sfr fields and command history section -also did you update the ground software side as well for the test?

[dhacks541]

Thanks for the thoughts, Duncan. In terms of the issue with the normal report, the extra field was added at the end of the other sfr fields and before the command history. With that being said, I'm sure that the issue was a mismatch between the update and what was reflected in the ground station - Jonathan is currently in Denmark so making updates to the GS software is not super easy on a short timescale.

In any case I agree with your assessment that even if we made the relatively simple changes to the normal report and GS side, I don't know that it is practical or wise to make such significant changes on the day of final upload. I think based on the feedback I am inclined to keep the camera::delay_count value as an SFR field (and update the eeprom constant, thanks for catching that!) without adding to the NR, although I agree this is not ideal. Just considering the benefit to the mission, the non-critical nature of this value, and the fact that it is already incorporated and testing with GS in the loop while a retest is not practical. However if you or Lauren feel differently, I am happy to move this to constants before merging, assuming there are no concerns about the rest of the changes.

[dhacks541]

Oh and in terms of the clang version, looks like I have version 16. Updating the version the pipeline uses didn't seem to fix the issue, are we fine just ignoring this?

[Duncan-McD]

Oh and in terms of the clang version, looks like I have version 16. Updating the version the pipeline uses didn't seem to fix the issue, are we fine just ignoring this?

Yeah fine to ignore - possible they using different format rules still but not worth the extra effort tbh

[Duncan-McD]

Thanks for the thoughts, Duncan. In terms of the issue with the normal report, the extra field was added at the end of the other sfr fields and before the command history. With that being said, I'm sure that the issue was a mismatch between the update and what was reflected in the ground station - Jonathan is currently in Denmark so making updates to the GS software is not super easy on a short timescale.

In any case I agree with your assessment that even if we made the relatively simple changes to the normal report and GS side, I don't know that it is practical or wise to make such significant changes on the day of final upload. I think based on the feedback I am inclined to keep the camera::delay_count value as an SFR field (and update the eeprom constant, thanks for catching that!) without adding to the NR, although I agree this is not ideal. Just considering the benefit to the mission, the non-critical nature of this value, and the fact that it is already incorporated and testing with GS in the loop while a retest is not practical. However if you or Lauren feel differently, I am happy to move this to constants before merging, assuming there are no concerns about the rest of the changes.

Alright I concur, I think this is the correct path forward given the time left and the value the ability to change the field gives

The rest of the changes look good to me