sifive / duh

👾 Design ∪ Hardware
https://observablehq.com/@drom/duh-intro
Apache License 2.0
72 stars 6 forks source link

report status of DUH document #67

Open drom opened 4 years ago

drom commented 4 years ago

Add duh status command to report the general status metric about the document:

component

The total number of:

adeelliaquat-lm commented 3 years ago

Hi, Recently, we started working on Duh and came across this issue. We were successfully able to modify duh and add a status command, which is run by “duh status ”. It would be great to get your feedback on our work, so the required improvements can be done. Picture 1 shows the result of duh status, Does It suffice ? or are we missing something? Moreover, do we want to display any additional details? like shown in pictures 2 and 3.

Picture 1: duh_metrics

Picture 2: ports

Picture 3: metrics

Ramlakshmi3733 commented 3 years ago

Love your work! thank you.

Some thoughts:
a) does it make sense to dump out the interfaces by name and type including VLNV b). When dumping out the interfaces, option by user to dump out the ports associated with each interfaces c) dump of parameters/properties with default values

Since JSON5/DuH is readable format, the above may seem unnecessary.

What are the management utilities that could be done with the capability written? a) A Component with very few interfaces is DuH-compliant but useless

adeelliaquat-lm commented 3 years ago

Thanks for the response, really appreciate your feedback. In case of additional information like ports, associated with each interface, a user option is a good idea but as you mentioned, displaying it may be unnecessary. So, should we proceed with the general status metric of the document, as shown in picture 1?

Love your work! thank you.

Some thoughts: a) does it make sense to dump out the interfaces by name and type including VLNV b). When dumping out the interfaces, option by user to dump out the ports associated with each interfaces c) dump of parameters/properties with default values

Since JSON5/DuH is readable format, the above may seem unnecessary.

What are the management utilities that could be done with the capability written? a) A Component with very few interfaces is DuH-compliant but useless

Ramlakshmi3733 commented 3 years ago

yes please. we would appreciate it greatly. The only enhancement i would request: Where you have "Bus Interfaces: 2", it would be great to list the type of interfaces and number [forget the port map]

adeelliaquat-lm commented 3 years ago

yes please. we would appreciate it greatly. The only enhancement i would request: Where you have "Bus Interfaces: 2", it would be great to list the type of interfaces and number [forget the port map]

Tried to do the required modifications. The figure is attached for your reference. Moreover, I have added a short description of some metrics. Would love to hear your comments on it.

bus interfaces: Shows the total no of interfaces, present in busInterfaces (in .json file) If the busInterfaces (in .json file) is not empty, it shows the following information:

Un-mapped Ports: It shows the total number of un-mapped ports. In the case of multiple portgroups in the busInterfaces, it shows the sum of un-mapped ports, from the selected mapping of each portgroup.

Registers: It shows the total number of registers

Fields: It shows the total number of register fields. In case of multiple registers, it shows the sum of fields of each register. Is this approach fine or do we want to specify fields per register?

duh_status

If the above needs any modification, let me know. Thanks

Ramlakshmi3733 commented 3 years ago

this is awesome! Could you please work out the details on how to check it in? I would gladly help build testcases for you, to ensure that if there are any issues we catch it before release.

drom commented 3 years ago

Looks great. In general, all ports should be mapped to busInterfaces (VLNV or bundles). I think it makes sense to make Un-mapped Ports number red when it is > 0.

Also, it makes sense to report a total width of all ports (except when the width is an expression). Or even total number of input and output bits:

Input width: 123 bits,
Output width: 2340 bits,

Please use the following guideline on filing the PR https://github.com/sifive/duh/blob/master/.github/CONTRIBUTING.md

adeelliaquat-lm commented 3 years ago

Looks great. In general, all ports should be mapped to busInterfaces (VLNV or bundles). I think it makes sense to make Un-mapped Ports number red when it is > 0.

Also, it makes sense to report a total width of all ports (except when the width is an expression). Or even total number of input and output bits:

Input width: 123 bits,
Output width: 2340 bits,

Please use the following guideline on filing the PR https://github.com/sifive/duh/blob/master/.github/CONTRIBUTING.md

Thanks for the suggestions. We've done the required modifications, which are as follows: 1: Un-mapped Ports are displayed in red if they’re greater than 0, else they’re displayed in white 2: The Calculated width of I/O ports is displayed

Samples are attached for your reference. Picture 1 shows a scenario where one or more expressions were present in port widths, while Picture 2 shows the one with no expression and some un-mapped ports.

Picture 1: Sample_1

Picture 2: Sample_2_noExp_Umap

adeelliaquat-lm commented 3 years ago

this is awesome! Could you please work out the details on how to check it in? I would gladly help build testcases for you, to ensure that if there are any issues we catch it before release.

Thanks for appreciating our efforts. For the test cases, we have populated a sheet (link given below), Please have a look at it and give your feedback. Your guidance will enable us to further mature duh status for PR.

Link : https://docs.google.com/spreadsheets/d/1-IEBhSii1OXSv9rkEgYabcqE85qlReMO-t4U_2tf-D4/edit?usp=sharing

adeelliaquat-lm commented 3 years ago

@drom @Ramlakshmi3733 As per above discussion, PR is pending. Your feedback is much appreciated.

Thanks