(De)Serialization clean up of temporary workarounds #3971

vincewoo · 2025-08-01T19:52:52Z


*Related Issue(s)*	#3944
*Has Unit Tests (y/n)*	y, pre-existing tests
*Documentation Included (y/n)*	n

Change Description

This change continues the effort for #3781. The FPP change has been completed to be compatible with the new serialization methods. This PR removes the temporary workarounds introduced by #3880 that we had in place to decouple with FPP.

Rationale

Cleaning up the temporary workarounds left that we had in place to decouple the necessary FPP upgrade completed by nasa/fpp#763.

Testing/Review Recommendations

No new UTs were added as part of this change. Existing were updated where necessary and re-run for regression.

Future Work

We still need to mark the legacy serialize and deserialize methods of Fw::Serializable and Fw::SerializeBufferBase as DEPRECATED.

…ackwards compat hacks. Updated some uses of the legacy functions that were missed originally.

… should be inherited now

…from SerializeStatus

…able.

LeStarch · 2025-08-01T22:28:20Z

@vincewoo I don't want the functions deprecated yet. Can we just remove the temporary variables? Next release we will DEPRECATE and then remote the release after that.

vincewoo · 2025-08-01T22:31:10Z

@vincewoo I don't want the functions deprecated yet. Can we just remove the temporary variables? Next release we will DEPRECATE and then remote the release after that.

Yes, I can do that!

Fw/Com/ComPacket.cpp


 SerializeStatus ComPacket::serializeBase(SerializeBufferBase& buffer) const {
-    return buffer.serialize(static_cast<FwPacketDescriptorType>(this->m_type));
+    return buffer.serializeFrom(static_cast<FwPacketDescriptorType>(this->m_type));


Fw/Com/ComPacket.cpp

 SerializeStatus ComPacket::deserializeBase(SerializeBufferBase& buffer) {
    FwPacketDescriptorType serVal;
-    SerializeStatus stat = buffer.deserialize(serVal);
+    SerializeStatus stat = buffer.deserializeTo(serVal);


Fw/Time/Time.cpp

Fw/Tlm/TlmPacket.cpp

Fw/Types/PolyType.cpp

    // get type
    FwEnumStoreType des;
-    SerializeStatus stat = buffer.deserialize(des);
+    SerializeStatus stat = buffer.deserializeTo(des);


Fw/Types/Serializable.cpp

Fw/Types/StringBase.cpp

-    return this->serializeTo(buffer, maxLength);
+// Deprecated method for backward compatibility
+SerializeStatus StringBase::serialize(SerializeBufferBase& buffer) const {
+    return this->serializeTo(buffer);


Fw/Types/StringBase.cpp

-    return this->deserializeFrom(buffer);
+// Deprecated method for backward compatibility
+SerializeStatus StringBase::serialize(SerializeBufferBase& buffer, SizeType maxLength) const {
+    return this->serializeTo(buffer, maxLength);


Fw/Types/StringBase.cpp

-    return this->deserializeFrom(buffer);
+// Deprecated method for backward compatibility
+SerializeStatus StringBase::serialize(SerializeBufferBase& buffer, SizeType maxLength) const {
+    return this->serializeTo(buffer, maxLength);


Fw/Types/StringBase.hpp

Fw/Time/Time.cpp

-        status = this->m_val.serialize(buffer);
-    }
-    return status;
+    return this->m_val.serializeTo(buffer);


Fw/Time/Time.cpp

-
-SerializeStatus Time::deserialize(SerializeBufferBase& buffer) {
-    return this->deserializeFrom(buffer);
+    return this->m_val.deserializeFrom(buffer);


Fw/Tlm/TlmPacket.cpp


    // id
-    SerializeStatus stat = this->m_tlmBuffer.serialize(id);
+    SerializeStatus stat = this->m_tlmBuffer.serializeFrom(id);


Fw/Tlm/TlmPacket.cpp


    // time tag
-    stat = this->m_tlmBuffer.serialize(timeTag);
+    stat = this->m_tlmBuffer.serializeFrom(timeTag);


Fw/Tlm/TlmPacket.cpp


    // id
-    SerializeStatus stat = this->m_tlmBuffer.deserialize(id);
+    SerializeStatus stat = this->m_tlmBuffer.deserializeTo(id);


Fw/Tlm/TlmPacket.cpp


    // time tag
-    stat = this->m_tlmBuffer.deserialize(timeTag);
+    stat = this->m_tlmBuffer.deserializeTo(timeTag);


Fw/Tlm/TlmPacket.cpp


    // telemetry buffer
-    stat = this->m_tlmBuffer.deserialize(buffer.getBuffAddr(), bufferSize, Fw::Serialization::OMIT_LENGTH);
+    stat = this->m_tlmBuffer.deserializeTo(buffer.getBuffAddr(), bufferSize, Fw::Serialization::OMIT_LENGTH);


Fw/Tlm/TlmPacket.cpp


    // telemetry buffer
-    stat = this->m_tlmBuffer.deserialize(buffer.getBuffAddr(), bufferSize, Fw::Serialization::OMIT_LENGTH);
+    stat = this->m_tlmBuffer.deserializeTo(buffer.getBuffAddr(), bufferSize, Fw::Serialization::OMIT_LENGTH);


Fw/Tlm/TlmPacket.cpp

 SerializeStatus TlmPacket::serializeTo(SerializeBufferBase& buffer) const {
    // serialize the number of packets
-    SerializeStatus stat = buffer.serialize(this->m_numEntries);
+    SerializeStatus stat = buffer.serializeFrom(this->m_numEntries);


Fw/Tlm/TlmPacket.cpp

 SerializeStatus TlmPacket::deserializeFrom(SerializeBufferBase& buffer) {
    // deserialize the number of packets
-    SerializeStatus stat = buffer.deserialize(this->m_numEntries);
+    SerializeStatus stat = buffer.deserializeTo(this->m_numEntries);


vincewoo · 2025-08-05T22:21:31Z

@thomas-bc Note that this PR is deleting all the serialize/deserialize methods from the child classes so they inherit properly from the parent now. This makes it easier for use to later deprecate as well.

Fw/Types/Serializable.hpp

Kronos3

fprime/Fw/Types/Serializable.hpp

Lines 39 to 41 in fe88650

    
           virtual SerializeStatus serialize(SerializeBufferBase& buffer) const; 
        
           virtual SerializeStatus deserialize(SerializeBufferBase& buffer);

I see serialize/deserialize are still virtual, we may not want this since they should not be overridden. Maybe making them inline in the header is better?

Fw/Types/StringBase.cpp

Fw/Types/Serializable.hpp

    // ----------------------------------------------------------------------

-    virtual SerializeStatus serialize(SerializeBufferBase& buffer) const;
+    SerializeStatus serialize(SerializeBufferBase& buffer) const { return this->serializeTo(buffer); }


Fw/Types/Serializable.hpp

-    virtual SerializeStatus serialize(SerializeBufferBase& buffer) const;
+    SerializeStatus serialize(SerializeBufferBase& buffer) const { return this->serializeTo(buffer); }
+
+    SerializeStatus deserialize(SerializeBufferBase& buffer) { return this->deserializeFrom(buffer); }


Fw/Types/Serializable.hpp

    // ----------------------------------------------------------------------

-    virtual SerializeStatus serialize(SerializeBufferBase& buffer) const;
+    SerializeStatus serialize(SerializeBufferBase& buffer) const { return this->serializeTo(buffer); }


Fw/Types/Serializable.hpp

-    virtual SerializeStatus serialize(SerializeBufferBase& buffer) const;
+    SerializeStatus serialize(SerializeBufferBase& buffer) const { return this->serializeTo(buffer); }
+
+    SerializeStatus deserialize(SerializeBufferBase& buffer) { return this->deserializeFrom(buffer); }


…Inherit from parent.

Kronos3

I see the virtual on serialize/deserialize has been addressed, looks great! Thanks for the fast turnaround on this!

bocchino · 2025-08-06T18:25:34Z

Looks good, I just had a few questions/comments. @LeStarch @thomas-bc can we remove the Deferred to Next Release label from this PR? I think it is close, and it's important for the release.

bocchino

Looks good!

LeStarch · 2025-08-06T18:50:26Z

Got verbal buy off from @thomas-bc and @bocchino . Good to merge.

vincewoo added 6 commits August 1, 2025 11:19

Staging DEPRECATE changes in preparation for new FPP alpha. Removed b…

cc3971b

…ackwards compat hacks. Updated some uses of the legacy functions that were missed originally.

Remove declaration of serialize and deserialize from RawTime as those…

fbf2415

… should be inherited now

Merge branch 'devel' into FP-3944

b72aa95

Removing FW_SERIALIZE_UNIMPLEMENTED and FW_DESERIALIZE_UNIMPLEMENTED …

50e63d2

…from SerializeStatus

Removing superfluous comma

c1f32a3

Missed AmpcsEvrLogPacket. Fixing a minor whitespace issue in Serializ…

6f3fb0f

…able.

vincewoo added 2 commits August 1, 2025 15:44

Un-deprecating serialize/deserialize for this release

0097e7d

Restoring DEPRECATED for the noLength functions

286d638

thomas-bc added the Deferred To Next Release This PR is marked as deferred until after the upcoming release. label Aug 2, 2025

vincewoo added 2 commits August 4, 2025 12:45

Merge branch 'devel' into FP-3944

0bf050b

Pulling in fpp 3.0.0a17. Fixing some lingering issues with refactoring.

cca2976

github-advanced-security bot found potential problems Aug 4, 2025

View reviewed changes

Fw/Types/StringBase.hpp Fixed Show fixed Hide fixed

Fw/Types/StringBase.hpp Fixed Show fixed Hide fixed

Fw/Types/StringBase.hpp Fixed Show fixed Hide fixed

thomas-bc and others added 5 commits August 4, 2025 13:43

Format files

68dd73c

Update fprime-fpp package

8cf22f0

Merge branch 'devel' into FP-3944

38c4702

Fixing some lingering issues with serialization modernization.

c686be0

Merge branch 'devel' into FP-3944

782d772

github-advanced-security bot found potential problems Aug 5, 2025

View reviewed changes

vincewoo added 4 commits August 5, 2025 14:04

Fixing weird merge issue with AmpcsEvrLogPacket

fa7ba9e

More clang issue fixes

30f62d9

Still pesky clang formatting issues. Superfluous whitespace.

73e43fe

Removing redundant virtuals for overriden methods

fe88650

vincewoo marked this pull request as ready for review August 5, 2025 22:02

vincewoo requested a review from thomas-bc August 5, 2025 22:02

vincewoo changed the title ~~Deprecate legacy serialize and deserialize methods. Cleaning up temporary workarounds.~~ (De)Serialization clean up of temporary workarounds Aug 5, 2025

vincewoo commented Aug 5, 2025

View reviewed changes

Fw/Types/Serializable.hpp Show resolved Hide resolved

thomas-bc requested review from Kronos3 and bocchino August 6, 2025 16:23

Kronos3 reviewed Aug 6, 2025

View reviewed changes

thomas-bc reviewed Aug 6, 2025

View reviewed changes

Fw/Types/StringBase.cpp Show resolved Hide resolved

Incorporating PR comments

5abe3d1

github-advanced-security bot found potential problems Aug 6, 2025

View reviewed changes

Fw/Types/Serializable.hpp Fixed Show fixed Hide fixed

Fw/Types/Serializable.hpp Fixed Show fixed Hide fixed

clang formatted Serializable.hpp

3972a41

github-advanced-security bot found potential problems Aug 6, 2025

View reviewed changes

Removing redundant serialize and deserialize methods in TestAbsType. …

c32ffca

…Inherit from parent.

vincewoo requested a review from thomas-bc August 6, 2025 18:10

Kronos3 approved these changes Aug 6, 2025

View reviewed changes

thomas-bc removed the Deferred To Next Release This PR is marked as deferred until after the upcoming release. label Aug 6, 2025

LeStarch approved these changes Aug 6, 2025

View reviewed changes

bocchino approved these changes Aug 6, 2025

View reviewed changes

thomas-bc approved these changes Aug 6, 2025

View reviewed changes

LeStarch merged commit f38010a into nasa:devel Aug 6, 2025
49 checks passed

vincewoo deleted the FP-3944 branch August 6, 2025 22:07

vincewoo mentioned this pull request Aug 6, 2025

After FPP updates to new v4.0 serialization methods, mark legacy serialization methods as DEPRECATED #3944

Closed

	virtual SerializeStatus serialize(SerializeBufferBase& buffer) const;

	virtual SerializeStatus deserialize(SerializeBufferBase& buffer);

(De)Serialization clean up of temporary workarounds #3971

(De)Serialization clean up of temporary workarounds #3971

Conversation

vincewoo commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change Description

Rationale

Testing/Review Recommendations

Future Work

Uh oh!

LeStarch commented Aug 1, 2025

Uh oh!

vincewoo commented Aug 1, 2025

Uh oh!

Check warning

Check warning

Uh oh!

Uh oh!

Uh oh!

Check warning

Uh oh!

Check warning

Check warning

Check warning

Uh oh!

Uh oh!

Uh oh!

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

vincewoo commented Aug 5, 2025

Uh oh!

Uh oh!

Kronos3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check warning

Check warning

Check notice

Check notice

Kronos3 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bocchino commented Aug 6, 2025

Uh oh!

bocchino left a comment

Choose a reason for hiding this comment

Uh oh!

LeStarch commented Aug 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

vincewoo commented Aug 1, 2025 •

edited

Loading

Kronos3 left a comment •

edited

Loading