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.

Sign up for GitHub

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

Jump to bottom

fix clang compilation and FPGA integration #293

Merged
merged 8 commits into from
May 13, 2024
Merged

fix clang compilation and FPGA integration #293

merged 8 commits into from
May 13, 2024

Conversation

n-eiling
Copy link
Contributor

This used to work before but the CI test was not migrated to github and therefore we lost the compatibility. We need clang compilation to make the clangd language server work.

@n-eiling
fix Interface::mOpened being unitilizated
808b404 
Signed-off-by: Niklas Eiling <niklas.eiling@eonerc.rwth-aachen.de>
@n-eiling
revert to an MNASolverFactory that also works without KLU
71375e6 
Signed-off-by: Niklas Eiling <niklas.eiling@eonerc.rwth-aachen.de>
@n-eiling
add villas interface FpgaExample
8c8056b 
Signed-off-by: Niklas Eiling <niklas.eiling@eonerc.rwth-aachen.de>
@n-eiling
add error logging to InterfaceWorkerVillas
3e30771 
Signed-off-by: Niklas Eiling <niklas.eiling@eonerc.rwth-aachen.de>
@n-eiling
fix illegal C++ that lead to compilation failing when using clang
1018ba7 
Signed-off-by: Niklas Eiling <niklas.eiling@eonerc.rwth-aachen.de>
@n-eiling
add clang compilation to CI
ea4b1db 
Signed-off-by: Niklas Eiling <niklas.eiling@eonerc.rwth-aachen.de>
@n-eiling n-eiling changed the title fix clang compilation. fix clang compilation and FPGA integration May 13, 2024
@n-eiling n-eiling marked this pull request as ready for review May 13, 2024 11:05
@n-eiling
Copy link
Contributor Author

n-eiling commented May 13, 2024 •
edited

I noticed a lot of errors in the models due to proper override semantics not being enforced. For example most clone implementations were not overriding but only hiding. Same for some log implementations.
I fixed all of what clang catches and also added a CI target to build using clang.
This makes clangd/intellisense work.

This also fixes a few minor issues with the VILLAS interface and adds a draft for an FPGA interface.

Let's merge this quickly, to avoid having to touch even more files. @stv0g @m-mirz

btw: We also lost the cppcheck CI. This was also catching a lot of the mistakes I had to fix here.
we should make the cppcheck CI more strict.

fixes #256 , #227

This was referenced May 13, 2024
Closed
Closed
@n-eiling
fix clang compiler errors.
cc4ab35 
there are some pretty significant mistakes that I fixed here. For example in most models clone was implemented such that it didn't properly used polymorphism.

Signed-off-by: Niklas Eiling <niklas.eiling@eonerc.rwth-aachen.de>
@leonardocarreras leonardocarreras mentioned this pull request May 13, 2024
Closed
@stv0g
Copy link
Contributor

stv0g commented May 13, 2024

LGTM. I also fixed the uninitialized mOpened member in another PR. But let's go ahead and merge this here.

@m-mirz I am in support of merging this. Can you pull the trigger?

stv0g
stv0g previously approved these changes May 13, 2024
View reviewed changes
@n-eiling
deactivate Pybind in clang build for now
acc959c 
Signed-off-by: Niklas Eiling <niklas.eiling@eonerc.rwth-aachen.de>
@n-eiling
Copy link
Contributor Author

Pybind is not working with clang because of some template error that are beyond me. I just deactivated pybind in the CI until somehow who has a better understanding of the attribute templates gets around to fixing it.

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented May 13, 2024

Quality Gate Passed Quality Gate passed

Issues
12 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
49.9% Duplication on New Code

See analysis details on SonarCloud

leonardocarreras
leonardocarreras reviewed May 13, 2024
View reviewed changes
Copy link
Collaborator

@leonardocarreras leonardocarreras left a comment

Choose a reason for hiding this comment

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

Looks good to me, many of the changes are 1 to 1 with #257

dpsim/include/dpsim/DiakopticsSolver.h Show resolved Hide resolved
dpsim/include/dpsim/MNASolverFactory.h Show resolved Hide resolved
@leonardocarreras leonardocarreras mentioned this pull request May 13, 2024
Merged
@leonardocarreras
Copy link
Collaborator

I suggest we can continue in #294, if everyone agrees

@m-mirz m-mirz merged commit f07066a into master May 13, 2024
22 checks passed
@m-mirz m-mirz deleted the villas-fpga branch May 13, 2024 19:41
m-mirz added a commit that referenced this pull request May 30, 2024
@m-mirz
Enable compilation in gcc 14 and clang 18 (code fixes) (#294)
518be00 
~~This integrates the changes in the #293 and #257 to have all in one
place.~~
Now #257 is rebased on the master (#293 merged).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@m-mirz m-mirz m-mirz approved these changes

@leonardocarreras leonardocarreras leonardocarreras left review comments

@stv0g stv0g stv0g left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@n-eiling @stv0g @leonardocarreras @m-mirz