Clarify thread_safe metadata for encoder component

[cleishm]

The .eil.yml for the encoder component has thread_safe: yes, but the API doesn't appear to be thread-safe in the traditional sense.

Functions like rotary_encoder_enable_acceleration() and rotary_encoder_disable_acceleration() mutate encoder state that is also read by the timer callback, without any synchronization. This has been the case since before the recent refactoring.

Should thread_safe be changed to no, or should synchronization be added to the public API functions? Happy to submit a PR either way.

[UncleRus]

Since we aim to keep the entry barrier for our components as low as possible, it would be better to make the relevant functions thread-safe.

[cleishm]

That's a fair point - and apologies, I should have been clearer about my preference in the original question. I think the right fix is to change thread_safe to no.

ESP-IDF's own peripheral drivers (gpio, i2c, spi, ledc, pcnt, etc.) don't internally synchronize per-handle configuration calls - the convention is that the application coordinates access to a given handle.

Adding a mutex here would also mean the timer callback has to acquire a lock on every polling cycle, introducing priority inversion risk in a timing-sensitive path.

Happy to add a doc note clarifying the thread safety expectations instead, and submit a PR either way.

[UncleRus]

Thank you for the detailed analysis and the thoughtful explanation — I really appreciate it.

You raise a valid point about the potential priority inversion and the overhead of acquiring a lock in the timer callback. I agree that it's better to keep the API as is and clearly document its thread-safety limitations rather than adding mutexes that could impact timing-sensitive paths.

So, changing thread_safe: no in .eil.yml and adding a note in the documentation (in Doxygen comments and possibly in the README) would be the right approach.

[cleishm]

After further consideration, I've added a FreeRTOS mutex in PR #8. There was a very small chance of an internal race between the timer callback and the API (e.g. a potential use-after-free on delete), which isn't something the caller can reasonably coordinate around, since the timer callback is internal to the library.

A task-level mutex avoids the interrupt-disabling overhead of a spinlock, and the priority inversion risk is minimal given the lock is only held briefly during the polling cycle.

That said, I'm not sure what thread_safe is meant to indicate here. The library API is still not fully thread-safe - for example, calling rotary_encoder_delete concurrently with other API methods on the same handle is undefined behavior, since delete destroys the lock and frees the handle. This is consistent with how most C resource APIs work (e.g. fclose/fread), but it does mean thread_safe: yes may be misleading.