search

Severson-Group / AMDC-Firmware

Embedded system code (C and Verilog) which runs the AMDC Hardware
http://docs.amdc.dev/firmware
BSD 3-Clause "New" or "Revised" License
30 stars 5 forks source link

Update Eddy Current Sensor IP from v1.0 to v2.0 #307

Closed codecubepi closed 12 months ago

codecubepi commented 1 year ago

This PR renames the Eddy Current Sensor from v1.0 to v2.0, to reflect the recent major changes made in this PR: #304 .

See the following comments:

https://github.com/Severson-Group/AMDC-Firmware/pull/304#pullrequestreview-1448348477

https://github.com/Severson-Group/AMDC-Firmware/pull/304#issuecomment-1574216229

I've updated both the REV E and REV D block designs to use the finished "2.0" IP, but if someone could test the functionality in the lab before merging, I think that would be good.

codecubepi commented 1 year ago

I saw that... wasn't sure what to do.

That's what was auto generated by Vivado (despite entering 2.0 as the version).

It is also what we have in the other IP repo(s) that are past v1.0:

https://github.com/Severson-Group/AMDC-Firmware/tree/v1.0.x/ip_repo%2Famdc_dac_2.0%2Fdrivers

I can change it if you want, it's currently as it is just for consistency.

npetersen2 commented 1 year ago

@codecubepi huh.... this seems like a Vivado bug to me...? Otherwise, I am not sure why you would want the drivers folder to indicate v1.0, unless those are under some other sort of versioning...?

We don't use the drivers/ folder anyway in the current structure of the AMDC firmware, so let's just ignore this and keep it consistent with how Vivado auto-generates it. We can rename things later if desired.

npetersen2 commented 1 year ago

Let's wait until we can get this tested in the lab before merging. I'll try to test it soon.

codecubepi commented 1 year ago

@codecubepi huh.... this seems like a Vivado bug to me...? Otherwise, I am not sure why you would want the drivers folder to indicate v1.0, unless those are under some other sort of versioning...?

We don't use the drivers/ folder anyway in the current structure of the AMDC firmware, so let's just ignore this and keep it consistent with how Vivado auto-generates it. We can rename things later if desired.

My thoughts exactly.

npetersen2 commented 12 months ago

I'm going to merge this, since we have some other FPGA work planned for this summer, and merging the bd file is painful since it has to be done manually.

If this change broke anything, we'll just fix it in another PR.