td-firmware / firmware / App / Modes

ModePostTreat.c

Clone Tools

Stats

Commits this week: 4
Total Committers: 1
Last Commit: 16:11
Commits this week: 4

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
Today
4 hrs ago

Vendor - TEL - Praneeth Bunne

LDT-1886: addressed code review comments

Options
    • -110
    • +112
    ./ModePostTreat.c
  2. … 2 more files in changeset.
Wednesday 22 Apr
2:05 pm

Sean Nash

Not necessary

Not necessary

2:05 pm

Sean Nash

This command should be given once on way out of prior state instead of every 50ms while in this state. We also need to zero the timer counter at same time so the timeout check below works.

This command should be given once on way out of prior state instead of every 50ms while in this state.
We also need to zero the timer counter at same time so the timeout check below works.

2:03 pm

Sean Nash

Response needs to be sent immediately (in message handler function). This is too late.

Response needs to be sent immediately (in message handler function). This is too late.

2:02 pm

Sean Nash

We shouldn't need to do this door check if you've required door be closed for the state. Call the doorClosedRequired( TRUE ) function on transition into a state that requires the door to be closed ...

We shouldn't need to do this door check if you've required door be closed for the state.
Call the doorClosedRequired( TRUE ) function on transition into a state that requires the door to be closed and call with ( FALSE ) on transition into a state that doesn't require door to be closed. When required = TRUE, the switch monitor will trigger the door open alarm for you.

1:57 pm

Sean Nash

Same organization comment that I had for post-tx mode. Let's keep functions related to each pre-tx state together separated by some kind of comment banner.

Same organization comment that I had for post-tx mode. Let's keep functions related to each pre-tx state together separated by some kind of comment banner.

1:56 pm

Sean Nash

Remove extra blank line.

Remove extra blank line.

1:56 pm

Sean Nash

Looks like there's plans for 4 post-tx states and we only have your auto eject state implemented here below. Since all of these 4 states will be implemented in this file, maybe we should organize t...

Looks like there's plans for 4 post-tx states and we only have your auto eject state implemented here below.
Since all of these 4 states will be implemented in this file, maybe we should organize them - keep functions related to each state together separated by some kind of comment banner.

1:53 pm

Sean Nash

Not necessary if you give retract command on way out of prior state.

Not necessary if you give retract command on way out of prior state.

1:52 pm

Sean Nash

I don't think this if statement is necessary. We wouldn't be in this state if the ejector didn't extend already. I think we just want to see if BP is home again (completed 1 revolution) so we know ...

I don't think this if statement is necessary. We wouldn't be in this state if the ejector didn't extend already.
I think we just want to see if BP is home again (completed 1 revolution) so we know when to stop and go to next state.

1:48 pm

Sean Nash

Give ejector retract command and zero timer counter here too.

Give ejector retract command and zero timer counter here too.

1:47 pm

Sean Nash

Not needed if you do this on way out of prior state.

Not needed if you do this on way out of prior state.

1:46 pm

Sean Nash

Start BP and zero timer counter here.

Start BP and zero timer counter here.

1:46 pm

Sean Nash

Not necessary if fault property of alarm is TRUE.

Not necessary if fault property of alarm is TRUE.

1:45 pm

Sean Nash

Not necessary if you do this on way out of prior state.

Not necessary if you do this on way out of prior state.

1:44 pm

Sean Nash

Remove ejector homed stuff. Not needed.

Remove ejector homed stuff. Not needed.

1:44 pm

Sean Nash

Give eject cmd and zero timer counter here.

Give eject cmd and zero timer counter here.

1:43 pm

Sean Nash

give BP home command and zero timer counter here.

give BP home command and zero timer counter here.

1:40 pm

Sean Nash

Not necessary. Just make alarm have fault property = TRUE.

Not necessary. Just make alarm have fault property = TRUE.

1:38 pm

Sean Nash

Again, ejector should have been retracted way back in pre-treatment mode. We shouldn't need to do it again.

Again, ejector should have been retracted way back in pre-treatment mode. We shouldn't need to do it again.

1:37 pm

Sean Nash

Shouldn't need this if statement. If we timeout, we want to stop the pump and alarm.

Shouldn't need this if statement. If we timeout, we want to stop the pump and alarm.

1:36 pm

Sean Nash

Better to home BP on your way out of previous state so you don't have to use a cmd issued flag. Ejector should already be retracted, so I see no reason to home the ejector here. You should also zer...

Better to home BP on your way out of previous state so you don't have to use a cmd issued flag.
Ejector should already be retracted, so I see no reason to home the ejector here.
You should also zero the autoEjectTimerCounter too.

1:32 pm

Sean Nash

Not necessary to request fault mode. Just set alarm fault property to TRUE.

Not necessary to request fault mode. Just set alarm fault property to TRUE.

1:31 pm

Sean Nash

This is way too late. UI s/w expects response immediately (so always send it in message handler function.)

This is way too late. UI s/w expects response immediately (so always send it in message handler function.)

1:30 pm

Sean Nash

rejectionReason should be set to none (zero) here too.

rejectionReason should be set to none (zero) here too.

1:30 pm

Sean Nash

You should send response regardless of success - so I think this should be moved to the very end of the function, outside of any if statements so that it always responds. You should set accept flag...

You should send response regardless of success - so I think this should be moved to the very end of the function, outside of any if statements so that it always responds. You should set accept flag and reject reason codes in these if statements so the response that gets sent will have the correct payload.

1:27 pm

Sean Nash

Declare locals at top of scope (so before memcpy). Also, when assigning to a BOOL, be explicit (i.e. TRUE or FALSE). You can use a ternary operation for this (e.g. BOOL b = <bool expr> ? TRUE : FAL...

Declare locals at top of scope (so before memcpy).
Also, when assigning to a BOOL, be explicit (i.e. TRUE or FALSE).
You can use a ternary operation for this (e.g. BOOL b = <bool expr> ? TRUE : FALSE

1:24 pm

Sean Nash

Does this message need a BOOL? Will UI ever send this message with a FALSE? Check with Nico on this.

Does this message need a BOOL? Will UI ever send this message with a FALSE? Check with Nico on this.

1:22 pm

Sean Nash

Add a space before )

Add a space before )

1:21 pm

Sean Nash

Set the fault property for this alarm to TRUE and then you don't need to request to go to fault mode - the alarm will do it for you. You should never have to request fault mode like this.

Set the fault property for this alarm to TRUE and then you don't need to request to go to fault mode - the alarm will do it for you. You should never have to request fault mode like this.