Snippets for AXI-Stream signals and generics#41
Snippets for AXI-Stream signals and generics#41valerionew wants to merge 2 commits intoVHDL-LS:masterfrom
Conversation
|
This makes assumptions about the signal port names that there would only ever be 1 master and 1 slave in a port map which isn’t always the case. Personally, I don’t like the idea of providing this because 1. The full AXI-S signal list is provided, 2. It forces user to have all caps for each port signal, 3. The names are not changeable by the user, 4. The generic width name is not changeable by the user. IMO this doesn’t belong in the tool itself and should instead be a personal snippet. |
|
While I see your points and i admit that this particular version of the snippet may be a bit too personal, i think there is room for improvement here towards a more general version of these snippets. As long as having many snippets does not interfere with the correct functioning of the software, why not adding more, as long as they are significant |
|
I agree with your point, AXI is common enough that I am okay with having helper snippets for it. However, I would like them to be general enough to support most use cases. I.e., the following things should be considered:
I'm not sure if I'm missing anything obvious. |
|
Also, per your commit about making tab spacing consistent, I would prefer not to align anything for the declaration. Some users use tabs, some use spaces and VSC doesn't convert |
|
I think that having AXI snippets (as well as snippets for other industry standard busses) would be a very good addition. Some comments though: I think that aligning could be done as long as it is done using spaces, e.g: With regards to coding style, there is also the matter of the usage of upper/lower case. While I personally avoid using UPPER_CASE for names except for generics/constants I know that there are shops where e.g. ports are always written in upper case (and to be fair, the AXI signals are defined in UPPER_CASE in the specification). It should also be noted that while the Another problem, albeit probably smaller, is the type used for the signals. 'std_logic(_vector)' is in my experience by far the most commonly used, however at my current work, we use 'std_ulogic(_vector)' to allow for (theoretically) faster simulation and the ability for the simulator to detect multiple drivers at compile time. Using generics to set the widths should also be optional. While it provides a single point of definition for the address and data widths, it does at the same time indicate to the user of the entity that they may be set freely, i.e. the design supports different widths which it may not do. Another use case for not using generics could be that the widths are defined using custom constants in a package I also think that the name/prefix of the interface, e.g. the For the full AXI interface, it should be enough with a snippet with all the AXI signals and the user will then have to remove the unused ones. With AXI5 being released, it might also make sense to add a "4" somewhere in the snippet name. The above requires the snippets to support a couple of customizations.
Some proposals for further discussion (not sure how VS Code treats identical names and upper/lower case in the prefix). One way to have upper/lower case variation in the identifiers would be to make two variants of each template, one with the prefix in lower case and one with the prefix in uppercase. Snippets
|
|
Thanks @Bochlin for your precious comment. How about we close this PR and open an issue to further discuss the most sensible implementation of the snippets? |
No description provided.