Pushoff keep alive on ack received

[xiazhvera]

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[xiazhvera]

The write timestamp can't be shared/singleton.

If I send 5 publishes at times 1, 2, 3, 4, 5. And then an ack comes back for the first one, the push out needs to be based on the first one's write stamp (1).

I dont think it matters, we only need to push off based on the "last" write time since that's when our "last transmission" happened.

[bretambrose]

The write timestamp can't be shared/singleton. If I send 5 publishes at times 1, 2, 3, 4, 5. And then an ack comes back for the first one, the push out needs to be based on the first one's write stamp (1).

I dont think it matters, we only need to push off based on the "last" write time since that's when our "last transmission" happened.

The write timestamp can't be shared/singleton. If I send 5 publishes at times 1, 2, 3, 4, 5. And then an ack comes back for the first one, the push out needs to be based on the first one's write stamp (1).

I dont think it matters, we only need to push off based on the "last" write time since that's when our "last transmission" happened.

It absolutely matters. If the connect is dropped at time 1.5 then the ping cannot be pushed out farther than that. Connection drops aren't instantaneous.

What we're trying to express is the following:

"I know the connection was valid at time T because the request I sent at time T received a response from the server."

Edit: Adding the followup discussion for completeness/documentation:

Consider this situation Keep alive is 60 time units. The connection path goes from a very slow network (the device in the field) before eventually reaching a fast network (internet backbone). It takes 20 time units total for a packet to go from one endpoint to another. So a publish on time 1 arrives at the other end on time 21. A response sent at time 21 arrives at time 41. Now simulate what happens when the client sends a steady stream of publishes, one every time unit: publish(1), publish(2), publish(3), etc... The client receives the ack for publish(1) at time 41. The connection could potentially have been dropped at a point anywhere after time 21 and the ack still make it back to the client. A connection drop on the fast part of the network after the ack squeaks through is easily possible and using 41 as the push-out time is not what we want to do.

[codecov-commenter]

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.02% :warning:

Comparison is base (b777be4) 82.22% compared to head (d962b9c) 82.20%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #314 +/- ## ========================================== - Coverage 82.22% 82.20% -0.02% ========================================== Files 20 20 Lines 8670 8678 +8 ========================================== + Hits 7129 7134 +5 - Misses 1541 1544 +3 ``` | [Files Changed](https://app.codecov.io/gh/awslabs/aws-c-mqtt/pull/314?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs) | Coverage Δ | | |---|---|---| | [source/client\_channel\_handler.c](https://app.codecov.io/gh/awslabs/aws-c-mqtt/pull/314?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs#diff-c291cmNlL2NsaWVudF9jaGFubmVsX2hhbmRsZXIuYw==) | `74.60% <100.00%> (+0.93%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/awslabs/aws-c-mqtt/pull/314/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=awslabs)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[xiazhvera]

Updated the unit tests. Also checkout s_test_mqtt_connection_ping_basic_scenario_fn for existing keep alive tests.