Skip to content

Conversation

@vishwamartur
Copy link

Related to #789

Add a new driver for the PIC32MX chip in FreeRTOS Plus TCP.

  • BufferAllocation_2.c
    • Include necessary headers for buffer allocation.
    • Implement buffer allocation functions for PIC32MX.
  • NetworkInterface_eth.c
    • Include necessary headers for Ethernet driver.
    • Implement Ethernet driver functions for PIC32MX.
  • CMakeLists.txt
    • Add CMake configuration for PIC32MX Ethernet driver.

Related to FreeRTOS#789

Add a new driver for the PIC32MX chip in FreeRTOS Plus TCP.

* **BufferAllocation_2.c**
  - Include necessary headers for buffer allocation.
  - Implement buffer allocation functions for PIC32MX.
* **NetworkInterface_eth.c**
  - Include necessary headers for Ethernet driver.
  - Implement Ethernet driver functions for PIC32MX.
* **CMakeLists.txt**
  - Add CMake configuration for PIC32MX Ethernet driver.
@vishwamartur vishwamartur requested a review from a team as a code owner November 2, 2024 04:49
Copy link
Member

@ActoryOu ActoryOu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! Left my comments here. Please also share your verification detail with us!

@@ -0,0 +1,634 @@
/*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason to not reuse the buffer allocation file at source/portable/BufferManagement?

Copy link
Contributor

@htibosch htibosch Nov 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for me
I don't remember the differences, but with some thinking it should be possible to merge the two files. Every "special version" of a module needs extra testing and maintenance.
Hein

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the feedback! There are no code changes in BufferAllocation_2.c—it has been revised once. Please let me know if further clarification is required. Otherwise, you can disregard this file.

Copy link
Member

@ActoryOu ActoryOu Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you simply use that file in source/portable/BufferManagement instead of creating a duplicattion in this PR?

@aggarg
Copy link
Member

aggarg commented Nov 4, 2024

@vishwamartur Please describe how you tested these changes.

@aggarg
Copy link
Member

aggarg commented Nov 7, 2024

@vishwamartur Please share the testing details. Would you also share the details about how you developed these changes.

@vishwamartur
Copy link
Author

@aggarg Thank you for your feedback!

At the moment, I have not been able to test these changes directly on a PIC32MX device. Unfortunately, I don't currently have access to the hardware needed for thorough testing. I'm considering options such as using PICSimLab, as it provides a simulated environment for PIC microcontrollers, though it may not fully replicate all aspects of the PIC32MX Ethernet functionality. Alternatively, I could put this PR on hold until I can complete testing with the actual device.

Let me know if you have any suggestions or if putting the PR on hold sounds reasonable.

@aggarg
Copy link
Member

aggarg commented Nov 10, 2024

Thank your for sharing! I am closing this PR for now. Please create a new one when you are done with the testing.

@aggarg aggarg closed this Nov 10, 2024
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.

4 participants