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

Expose Internal Functions for Enhanced Usability #167

Merged
merged 6 commits into from
Nov 8, 2023
Merged

Conversation

cryi
Copy link
Contributor

@cryi cryi commented Oct 25, 2023

Description

In this Work-In-Progress (WIP) PR, we have showcased a sample implementation that exposes the internal functions sendHttpHeaders, sendHttpData, and receiveAndParseHttpResponse within the coreHTTP library. Exposing these functions aims to provide developers with more control over the HTTP communication process, enabling the support for chunked body reads and writes, streaming, and other advanced HTTP functionalities which are essential for building more sophisticated applications atop coreHTTP.

Test Steps

To reproduce the changes, follow these steps:

  1. Clone the branch associated with this PR.
  2. Navigate to the coreHTTP library directory.
  3. Compile and run the tests provided to observe the enhanced functionality enabled by exposing the mentioned internal functions.

Checklist:

  • I have tested my changes. No regression in existing tests.
  • I have added unit-tests to cover the new functionality introduced in this Pull Request.

Related Issue

#166
#160

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@cryi cryi changed the title Title: Expose Internal Functions for Enhanced Usability WIP: Expose Internal Functions for Enhanced Usability Oct 25, 2023
@archigup
Copy link
Member

archigup commented Nov 2, 2023

Thanks for the PR!

Seems good, though should rename the existing functions to HTTPClient_SendHttpHeaders, HTTPClient_SendHttpData, etc., instead of adding a wrapper function.

Seems that in CI, unit tests are hitting an issue since there are "^M" characters at the end of two of the lines.

The actions have a formatting patch you can apply, or I can do so for you if you would prefer.

With the above, the only thing left would be to add the doc comments to the functions for doxygen, which we can help with if needed.

@cryi
Copy link
Contributor Author

cryi commented Nov 3, 2023

/bot run formatting

exposed internal function, added areHeadersComplete
@cryi
Copy link
Contributor Author

cryi commented Nov 4, 2023

Hello @archigup,

I've updated the functions and added pResponse->areHeadersComplete. We occasionally encounter insufficient memory during production testing and previously couldn't determine if the headers were complete. I've also run the formatting as requested. Regarding the documentation, it would be helpful if you could manage that.

@ericbj29
Copy link

ericbj29 commented Nov 7, 2023

Hello @cryi,

Could you enable maintainers to edit this pull request so I can add documentation?

@cryi
Copy link
Contributor Author

cryi commented Nov 7, 2023

Done @ericbj29 . (I had to move repository from org because of #5634)

@archigup archigup changed the title WIP: Expose Internal Functions for Enhanced Usability Expose Internal Functions for Enhanced Usability Nov 8, 2023
@ericbj29
Copy link

ericbj29 commented Nov 8, 2023

Thank you, @cryi.

I have added the documentation and updated the code size and CBMC.

@archigup archigup merged commit 668db4b into FreeRTOS:main Nov 8, 2023
11 checks passed
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

Successfully merging this pull request may close these issues.

3 participants