VILLASframework / node

Connecting real-time power grid simulation equipment
https://fein-aachen.org/projects/villas-node/
Apache License 2.0
10 stars 5 forks source link

Use same memory accessor for FPGA memory write and read #790

Closed IgnoreWarnings closed 2 months ago

IgnoreWarnings commented 3 months ago

Fpga uses different write and read accessors, which leads to erroneous data.

n-eiling commented 3 months ago

ok I get why this is an issue for you. However, when using the Dino IP and it's digital to analog convert we need to send an integer between 0 and 0xFFFF which corrensponds to -10V to 10V. I should probably do the conversion in the FPGA but I have to think about if that really makes sense. Anyway doing the conversion in write is probably the wrong place.

For this PR, can you also remove the code below your change and only do the double to float conversion?

IgnoreWarnings commented 3 months ago

ok I get why this is an issue for you. However, when using the Dino IP and it's digital to analog convert we need to send an integer between 0 and 0xFFFF which corrensponds to -10V to 10V. I should probably do the conversion in the FPGA but I have to think about if that really makes sense. Anyway doing the conversion in write is probably the wrong place.

For this PR, can you also remove the code below your change and only do the double to float conversion?

I we can also sent uint32 or any other type, but i want read and write functions to operate on the same type. Maybe change both to uint32?

n-eiling commented 3 months ago

I changed the FPGA bitstream yesterday to convert the floats in the FPGA. This way our interface to the FPGA is always floats, which makes more sense i guess. This is also how the commercial hardware does it. Let me test this and then we can merge your changes.

IgnoreWarnings commented 2 months ago

Fixed by 323c77b58ffd2bcd33370ea917252e8c32cb900d . @n-eiling I think the pipeline still doesnt work with external branches?

IgnoreWarnings commented 2 months ago

Fixed in #793