[Matt Leese]

It's definitely safer to turn off interrupts when reading or writing from goal_velocity. The reason to turn off interrupts is that it's possible that while accessing the data, an interrupt may trigger and also access the data. It's then possible for incorrect data to be generated.

However, you really only need to disable interrupts if both regular code and an interrupt handler are writing to the memory location. In most cases, if your interrupt is writing the data and the regular code is reading it, you shouldn't need to disable interrupts.

As for setting it volatile, volatile is used when a variable may change in another thread of execution (not aplicable here) or in an interrupt handler. The reason for this is that the compiler could optimize out code to check for a value if the compiler thinks the variable couldn't be changed within that code path; but the variable could change the interrupt. I doubt this is particularly relevant here as you probably aren't sitting in a loop waiting for something to happen in your code. I'm also doubting the optimizing ability of this compiler (although I could be wrong). Either way, you can always be safe by declaring it that way.

Matt