Кодревью master 88db70ede8 #3

Open
opened 2026-04-08 18:09:24 +03:00 by mysol93 · 1 comment

в мастере вижу ряд проблем, которые лучше исправить.

  1. Старайтесь не использовать константы напрямую, т.е. так называемые magic numbers, когда нужно будет вернуться к коду поосле перерыва вы будете долго вспоминать откуда они взялись. Вводи где возможно parameters, где невозможно - localparam. Пример из файла generator.sv
		вот это неоч
		   |
           |
           V
    input [31:0]           pulse_width,
    input [31:0]           pulse_period,

Сделайте 
.
.
.
	parameter int PULSE_WIDTH_MAX = 100000
	parameter int PULSE_PERIOD_MAX = 100000)
(
.
.
.
    input wire [$clog2(PULSE_WIDTH_MAX)-1:0]   pulse_width,
    input wire [$clog2(PULSE_PERIOD_MAX)-1:0]  pulse_period,

В этом же файле

wire start_pulse = start & ~start_d; <=== фу, давайте всё присваивать в always/initial блоке или assign

В этом же файле cnt_pulse_num и cnt_period изменяют свои значения в разных if-statements. Ниоч хорошо, можно легко сломать логику отправив 2 старта подряд. Давайте так не делать.

К файлу sampler.sv Тоже есть вопрос вот тут

if (data_in == 12'b100000000000) begin <=== тут правильное условие для data_in?
  1. По тестам. Прошу увеличивать покрытие тестов путём генерации случайных чисел для воздействия на входы. Увеличивать их количество, количество итераций, а так же необходимо сравнение выходов модулей и проверка этих выходов. Т.е., например, если мы на выходе DUT получаем данныые, то мы должны сгененрировать референсные данные и сравнить результаты. В случае совпадения сделать вывод о верности теста в стиле PASS/FAIL. Тесты должны быть автоматические, т.е. запустил - получил результат. В случае отрицательного результата нужно иметь возможность изучить отладочную информацию (вейвформы, логи и т.д.)
  2. Зайлете скрипты запуска тестов. Список файлов для тестов можете внести в файл-листы.
[в мастере](https://git.radiophotonics.ru/baulin.fa/reflectometer_fpga_project/commit/88db70ede84b169ef9bfe4e9d41290dbe1b876ec) вижу ряд проблем, которые лучше исправить. 1) Старайтесь не использовать константы напрямую, т.е. так называемые magic numbers, когда нужно будет вернуться к коду поосле перерыва вы будете долго вспоминать откуда они взялись. Вводи где возможно parameters, где невозможно - localparam. Пример из файла [generator.sv](https://git.radiophotonics.ru/baulin.fa/reflectometer_fpga_project/commit/88db70ede84b169ef9bfe4e9d41290dbe1b876ec#diff-49ad991c18d616ce195a17c128450e5f7a949f86) ``` вот это неоч | | V input [31:0] pulse_width, input [31:0] pulse_period, Сделайте . . . parameter int PULSE_WIDTH_MAX = 100000 parameter int PULSE_PERIOD_MAX = 100000) ( . . . input wire [$clog2(PULSE_WIDTH_MAX)-1:0] pulse_width, input wire [$clog2(PULSE_PERIOD_MAX)-1:0] pulse_period, ``` В этом же файле ``` wire start_pulse = start & ~start_d; <=== фу, давайте всё присваивать в always/initial блоке или assign ``` В этом же файле **cnt_pulse_num** и **cnt_period** изменяют свои значения в разных if-statements. Ниоч хорошо, можно легко сломать логику отправив 2 старта подряд. Давайте так не делать. К файлу [sampler.sv](https://git.radiophotonics.ru/baulin.fa/reflectometer_fpga_project/commit/ad6d6a4e2b1550e715e56b6a33e00eded1630e73#diff-ec68d2f12e960ca117c4ea1287cbbf0808e314ba) Тоже есть вопрос вот тут ``` if (data_in == 12'b100000000000) begin <=== тут правильное условие для data_in? ``` 2) По тестам. Прошу увеличивать покрытие тестов путём генерации случайных чисел для воздействия на входы. Увеличивать их количество, количество итераций, а так же необходимо сравнение выходов модулей и проверка этих выходов. Т.е., например, если мы на выходе DUT получаем данныые, то мы должны сгененрировать референсные данные и сравнить результаты. В случае совпадения сделать вывод о верности теста в стиле PASS/FAIL. Тесты должны быть автоматические, т.е. запустил - получил результат. В случае отрицательного результата нужно иметь возможность изучить отладочную информацию (вейвформы, логи и т.д.) 3) Зайлете скрипты запуска тестов. Список файлов для тестов можете внести в файл-листы.

(╯°□°)╯︵ ┻━┻

(╯°□°)╯︵ ┻━┻
Sign in to join this conversation.
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: baulin.fa/reflectometer_fpga_project#3
No description provided.