Baro: Initial R3 Driver Implementation

[266-750Balloons]

Description

Implements support for the MS5607-02BA03 barometer on the revision 3 Flight Computer. Not yet finished, currently a draft.

The way a read works in this implementation is it goes through a finite state machine to:

1. Initiate internal baro conversion for pressure.
2. Initiate the timeout required by the datasheet for pressure conversion.
3. Run the ADC read for pressure.
4. Initiate internal baro conversion for temperature.
5. Initiate the timeout required by the datasheet for temperature conversion.
6. Run the ADC read for temperature.
7. Move the FSM to the successful terminal state.

The getter function will then use the conversion factors to convert to useful float values (not yet implemented).

Issue Link

#8

Also see corresponding firmware PR, SunDevilRocketry/Flight-Computer-Firmware#303, which has examples of the interrupt handlers that will be needed for this.

Testing

• Passes existing unit tests
• Unit tests modified
• Integration test performed

Attach any test artifacts here, if relevant.

Other

Leave any additional notes here

Reviewer Checklist

Standards

• Follows FCF Architectural Standards
• Follows SDR Coding Standards
• Code complexity/function Size is minimized
• Code is testable
• Code is readable and commented properly
• License terms are respected

Error Handling

• Potentially unsafe functions return a status code
• Error returns properly handled

Memory

• Stack allocated memory is scoped correctly
• Heap allocated memory is avoided
• Globally allocated memory is minimized except when necessary
• Pointers are used correctly
• Concurrency has been considered

Performance

• Rate limiters are respected
• Busy waiting is avoided
• "Delay" calls are not used in performance sensitive code

[ETSells]

thoughts on the first cut

[ETSells]

has this always been a stub? need to fix

[266-750Balloons]

As far as I can tell.

[266-750Balloons]

Note to self: commit is for baro_get_altitude.

The context is less clear because I finally fixed the white space f*** up.

[ETSells]

We definitely want to get rid of it bc the actual conversion is done by sensor.

[ETSells]

are these really the only settings for rev 3? id assume much lower oversampling rates are possible

[266-750Balloons]

As far as I can tell. At the very least, there are all that's mentioned in the datasheet.

[ETSells]

nit: i know this is a draft, but this is already hard to read due to divergence from SDR's coding standards

[266-750Balloons]

Would this be better:

switch ( baro_read_state )
    {
    case BARO_READ_FAIL:

    case BARO_READ_DONE: // No currently running read
        baro_read_state = BARO_CONV_PRESSURE;
        baro_IT_handler(); // Keep FSM logic in that function
        break;

    default: // Read in progress
        return BARO_BUSY;
    }
}

If not, what in particular is the issue?

[ETSells]

oh god this really isn't that bad, i was just way too tired to be doing a review at the time, sorry drew.

yes, the suggested version is good, although having FAIL fallthrough to DONE feels like it'll be an issue.

[ETSells]

cr: Do not call an ISR from a blocking context. Ideally, you abstract the FSM logic out to a shared function, and then this start function & the ISR can call into it.

[266-750Balloons]

Do you mean don't call the ISR from a blocking context?

[ETSells]

Basically what I'm trying to do here is make sure anything that calls itself an "interrupt handler" is safe from race conditions arising from lack of consideration of re-entrancy. If it says "*_IT_handler" I expect it'll only be called from the context of an ISR from the NVIC that can only be triggered once at a time. If we enter this from a blocking context and then the ISR triggers, we could be in the middle of modifying state and end up with a really nasty to debug race condition.

[ETSells]

An ISR will never be able to report its own status back to the main thread via return. You'll need to do this via a static.

[266-750Balloons]

I don't think this is actually used for anything. It's mostly because that's what was already in the header file and the original implementation. Should we just make the handler have a void return type, in both the original and new implementations?

[ETSells]

how long is this delay? on second thought, this might actually need microsecond precision which means the scheduler wouldn't work for it

[266-750Balloons]

pressure_timeout and temperature_timeout will be set by baro_init based on the OSR (as I don't want to calculate them at runtime); the delay depends on the sample rate. Here's the table from the datasheet.

I think I've found a way to get the precision we need. You can just do an output capture interrupt using the channels feature of our existing microsecond timer. It has 4 channels for output capture, and I don't think we're using them if I understand correctly.

[ETSells]

this state should not be entered during normal execution. consider debug asserting if it is hit.

[266-750Balloons]

Alright. Will do.