Skip to content

Added PHY drivers to CPP#6

Open
pradyunnkale wants to merge 30 commits intoPurdue-Space-Program:mainfrom
pradyunnkale:feature/phy_cpp_driver
Open

Added PHY drivers to CPP#6
pradyunnkale wants to merge 30 commits intoPurdue-Space-Program:mainfrom
pradyunnkale:feature/phy_cpp_driver

Conversation

@pradyunnkale
Copy link
Copy Markdown
Collaborator

No description provided.

@pradyunnkale pradyunnkale force-pushed the feature/phy_cpp_driver branch from fe8bda9 to c8144c6 Compare August 30, 2025 15:09
Comment thread drivers/phy/phy.cpp Outdated
Comment thread drivers/phy/phy.cpp Outdated
Comment thread drivers/phy/phy.cpp Outdated
Comment thread drivers/phy/phy.cpp Outdated
Comment thread drivers/phy/phy.h Outdated
Comment thread drivers/phy/phy.h Outdated
Copy link
Copy Markdown
Contributor

@CBL17 CBL17 left a comment

Choose a reason for hiding this comment

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

This needs to build before I continue reviewing it.

Comment thread drivers/CMakeLists.txt Outdated
Comment thread drivers/phy/phy.h Outdated
Comment thread drivers/phy/phy.h Outdated
Comment thread simulations/CMakeLists.txt Outdated
Comment thread simulations/phy/phy.h Outdated
@CBL17 CBL17 mentioned this pull request Sep 3, 2025
@CBL17 CBL17 requested a review from 0xMihir September 3, 2025 20:04
Copy link
Copy Markdown
Contributor

@CBL17 CBL17 left a comment

Choose a reason for hiding this comment

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

Please close out the threads once you take care of the issues.

Comment thread simulations/CMakeLists.txt Outdated
Comment thread drivers/phy/phy.h Outdated
Copy link
Copy Markdown
Member

@0xMihir 0xMihir left a comment

Choose a reason for hiding this comment

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

I think we should first test this driver with the real chip on the Nucleo board before creating the sim library. This way, we can understand the actual functionality of the device before testing.

Comment thread drivers/phy/phy.cpp Outdated
Comment thread drivers/phy/phy.cpp Outdated
Comment thread drivers/phy/phy.cpp Outdated
Comment thread drivers/phy/phy.cpp Outdated
Comment thread drivers/phy/phy.cpp Outdated
Comment thread api/phy_base.h Outdated
Comment thread drivers/phy/phy.h Outdated
Comment thread drivers/phy/phy.h Outdated
Comment thread drivers/CMakeLists.txt
target_link_libraries(${name} stm_hal cmsis)
endfunction()

if(NOT "${TARGET}" STREQUAL NATIVE)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should test this driver with the actual chip before writing tests, since we need some source of truth.

Comment thread simulations/phy/phy.cpp Outdated
#include "phy.h"


PHY_Base_Driver::ErrorCodes PHY_SIM_Driver::init(void) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Echoing my previous comment, we need to have a source of truth for these methods. We can't just have an empty method returning OK, since we need to know if our implementation is correct.

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