hdfirmware

firmware

Clone Tools

Stats

Commits this week: 0
Total Committers: 17
Last Commit: 19 Nov 25
Commits this week: 0

Lines of code count not available

Commit Activity

Commits By Day In Week
52 week commits volume
Commits By Day In Week
Commits by day
Commits By Day In Week
Commits by hour
Commits By Hour In Day
Commit calendar
 

Most active committers (90 days)

loading
loading
  • last updated a few seconds ago
All Commits Reviews
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
Tuesday 13 Apr 2021
2:33 pm

Sean Nash

DEN-7605: Fixed typo.

Options
    • -4
    • +4
    ./App/Services/AlarmMgmt.c
    • -1
    • +1
    ./App/Services/AlarmMgmt.h
    • -1
    • +1
    ./App/Services/SystemCommMessages.c
10:07 am

qnguyen  (deleted user)

The clearAlarmCondition() function will check if the alarm is active or not and only clears if the alarm has been raised.

The clearAlarmCondition() function will check if the alarm is active or not and only clears if the alarm has been raised.

8:57 am

Dara Navaei

Typecasting the variables is not needed here. The macro typecasts them.

Typecasting the variables is not needed here. The macro typecasts them.

8:52 am

Sean Nash

Should probably go to normal state on fail as well - otherwise you get stuck here in self-test state.

Should probably go to normal state on fail as well - otherwise you get stuck here in self-test state.

8:52 am

Dara Navaei

Can we clear this alarm when it is raised? If no, do we need this else statement? Because if the alarm has not been raised, we do not need to clear it either.

Can we clear this alarm when it is raised? If no, do we need this else statement? Because if the alarm has not been raised, we do not need to clear it either.

8:51 am

Sean Nash

I think better to use self test status enum (mentioned in .h comment). This enum will have pass/fail/in progress. I think if we're still in INIT, zero, or self-test state, in-progress is appropriat...

I think better to use self test status enum (mentioned in .h comment). This enum will have pass/fail/in progress. I think if we're still in INIT, zero, or self-test state, in-progress is appropriate return value. I think you should have a BOOL variable defined to remember whether self-test passed or failed (instead of inferring from current state). Then if in normal state, return pass if self-test passed, failed if not.

8:43 am

Dara Navaei

Recommend typecasting bloodLeakState to (U32)bloodLeakState. Outside of this driver the enums are only U32s until they are converted to states again in Dialin.

Recommend typecasting bloodLeakState to (U32)bloodLeakState. Outside of this driver the enums are only U32s until they are converted to states again in Dialin.

8:43 am

Sean Nash

Same comment as above re: timeout.

Same comment as above re: timeout.

8:42 am

Sean Nash

Should AND this bit off from register so we don't interfere with other bits that may have been set by other drivers (e.g. bubble detector). Should be something like fpgaActuatorSetPoints.fpgaSensor...

Should AND this bit off from register so we don't interfere with other bits that may have been set by other drivers (e.g. bubble detector).
Should be something like fpgaActuatorSetPoints.fpgaSensorTest &= ~FPGA_BLOOD_LEAK_SELF_TEST_CMD;

8:41 am

Sean Nash

Should AND this bit off from register so we don't interfere with other bits that may have been set by other drivers (e.g. bubble detector). Should be something like fpgaActuatorSetPoints.fpgaSensor...

Should AND this bit off from register so we don't interfere with other bits that may have been set by other drivers (e.g. bubble detector).
Should be something like fpgaActuatorSetPoints.fpgaSensorTest &= ~FPGA_BLOOD_LEAK_ZERO_CMD;

8:40 am

Sean Nash

Should OR ( |= instead of = ) this bit into register so we don't interfere with other bits that may have been set by other drivers (e.g. bubble detector).

Should OR ( |= instead of = ) this bit into register so we don't interfere with other bits that may have been set by other drivers (e.g. bubble detector).

8:39 am

Sean Nash

Should OR ( |= instead of = ) this bit into register so we don't interfere with other bits that may have been set by other drivers (e.g. bubble detector).

Should OR ( |= instead of = ) this bit into register so we don't interfere with other bits that may have been set by other drivers (e.g. bubble detector).

8:36 am

Sean Nash

Consider checking for the Zero each time we execute this state (not just after timeout). Consider moving timeout to the alarm below (give zero time to happen, alarm if timeout). Consider giving ala...

Consider checking for the Zero each time we execute this state (not just after timeout).
Consider moving timeout to the alarm below (give zero time to happen, alarm if timeout).
Consider giving alarm a little more than 10 ms (maybe 20 or 30ms) as 10ms is how long it will take to go from init to zero state (i.e. 10ms timeout will occur as soon as we get here).

8:29 am

Sean Nash

I would say this function requests that the blood leak detector be zeroed (and self-tested). Zero does not happen right here in this function.

I would say this function requests that the blood leak detector be zeroed (and self-tested). Zero does not happen right here in this function.

8:26 am

Sean Nash

There is a SELF_TEST_STATUS_T enum (pass/fail) defined in fwcommon Common.h. Consider returning this type instead of BOOL.

There is a SELF_TEST_STATUS_T enum (pass/fail) defined in fwcommon Common.h. Consider returning this type instead of BOOL.

Monday 12 Apr 2021
7:44 pm

pmontazemi  (deleted user)

Addressed.

Addressed.

7:44 pm

pmontazemi  (deleted user)

Addressed.

Addressed.

7:37 pm

pmontazemi  (deleted user)

Addressed.

Addressed.

7:37 pm

pmontazemi  (deleted user)

Addressed.

Addressed.

7:37 pm

pmontazemi  (deleted user)

Addressed.

Addressed.

7:37 pm

pmontazemi  (deleted user)

Addressed.

Addressed.

7:37 pm

pmontazemi  (deleted user)

Addressed.

Addressed.

7:36 pm

pmontazemi  (deleted user)

Addressed.

Addressed.

7:36 pm

pmontazemi  (deleted user)

Addressed.

Addressed.

7:36 pm

pmontazemi  (deleted user)

Addressed.

Addressed.

7:35 pm

pmontazemi  (deleted user)

Addressed.

Addressed.

7:35 pm

pmontazemi  (deleted user)

Addressed.

Addressed.

7:35 pm

pmontazemi  (deleted user)

Addressed.

Addressed.

7:34 pm

pmontazemi  (deleted user)

Addressed.

Addressed.

7:33 pm

Peman Montazemi <pmontazemi@diality.com>

DEN-7395: Addressed code review comments

Options
    • -105
    • +114
    ./App/Controllers/BloodLeak.c
    • -16
    • +2
    ./App/Controllers/BloodLeak.h
    • -14
    • +6
    ./App/Modes/SelfTests.c
    • -17
    • +46
    ./App/Services/FPGA.c
    • -1
    • +4
    ./App/Services/FPGA.h
    • -2
    • +2
    ./App/Services/SystemComm.c
    • -18
    • +10
    ./App/Services/SystemCommMessages.c
    • -3
    • +3
    ./App/Services/SystemCommMessages.h