Activity dgfirmware/firmware/App/Services/SystemCommMessages.c

5:50 pm

Sean Nash

More
Re: DG-DEN-9054-1
commented on review

Where did this go?

5:15 pm

Sean Nash

More
Re: DG-DEN-9054-1
commented on review

So these & operators are going to yield a zero or a non-zero number that will be different for each switch (not a boolean). I think your intent is for these switch data to be an enum. So, for examp...

So these & operators are going to yield a zero or a non-zero number that will be different for each switch (not a boolean). I think your intent is for these switch data to be an enum. So, for example, this one should be "... = ( ( fpgaRegister & CONCENTRATE_CAP_SWITCH_BIT_MASK ) != 0 ? OPEN : CLOSED );

5:13 pm

Sean Nash

More
Re: DG-DEN-9054-1
commented on review

Put test functions at bottom of module (after usual banner).

5:04 pm

Sean Nash

More
Re: DG-DEN-9054-1
commented on review

I don't see this being initialized in init function.

4:16 pm

Sean Nash

More
Re: DG-DEN-9054-1
commented on review

I like to add a comment to right of system library includes explaining why they are needed (i.e. which math functions are you using in this module).

10:31 am

Sean Nash

More
Re: DG-DEN-9054-1
commented on review

Should we close both drain valves VRd1 and VRd2 here?

10:24 am

Sean Nash

More
Re: DG-DEN-9054-1
commented on review

I think other fill volumes would come from HD, so we can remove this TODO.

10:21 am

Sean Nash

More
Re: DG-DEN-9054-1
commented on review

The GPIO register is shared (e.g. see fluid leak in above function). So I think we should have separate functions for each bit that we use in the GPIO register. Also - why does the function name ha...

The GPIO register is shared (e.g. see fluid leak in above function). So I think we should have separate functions for each bit that we use in the GPIO register.
Also - why does the function name have the word Count in it?

10:17 am

Sean Nash

More
Re: DG-DEN-9054-1
commented on review

Please restore the vector length test.

10:12 am

Sean Nash

More
Re: DG-DEN-9054-1
commented on review

How is this used?

10:10 am

Sean Nash

More
Re: DG-DEN-9054-1
commented on review

I think these @param descriptions could fit in one line for a cleaner look.

10:07 am

Sean Nash

More
Re: DG-DEN-9054-1
commented on review

For consistency, can we put Dialin functions at end?

10:00 am

Sean Nash

More
Re: DG-DEN-9054-1
commented on review

Compatibility check should be before FPGA test at end where it was. We can't do it this early because UI may not have provided us with version/compatibility info yet.

10:39 am