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

ppl: save constraints in odb at each call of set_io_pin_constraint command #6692

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

eder-matheus
Copy link
Contributor

No description provided.

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Signed-off-by: Eder Monteiro <[email protected]>
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Collaborator

@oharboe oharboe left a comment

Choose a reason for hiding this comment

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

What are you asking the user to do here and what happens if they don't heed this advice?

I think an error message is better...

"It is strongly recommended to set the mirrored pins before any other constraint, in order to have them properly stored into the database. "

@gadfort
Copy link
Collaborator

gadfort commented Feb 12, 2025

@eder-matheus wouldn't it be better to process the mirror constraints first and then then remainder if order matters?

@eder-matheus
Copy link
Contributor Author

@gadfort @oharboe I've updated the code to not depend on the order we call the commands. It should be able to properly handle multiple mirrored pins and region constraint calls, independently of what is called first.

Some unit tests were added to depict some different scenarios, and the result is the expected in all of them.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@oharboe
Copy link
Collaborator

oharboe commented Feb 12, 2025

@gadfort @oharboe I've updated the code to not depend on the order we call the commands. It should be able to properly handle multiple mirrored pins and region constraint calls, independently of what is called first.

Some unit tests were added to depict some different scenarios, and the result is the expected in all of them.

This sounds great! Sounds like I don't have to think about this anymore :-)

It is easier to write code and automated tests than documentation that builds a strong mental model with the user so that they can follow an exact recipe.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Signed-off-by: Eder Monteiro <[email protected]>
Signed-off-by: Eder Monteiro <[email protected]>
@eder-matheus
Copy link
Contributor Author

@maliberty This is ready for review.

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