drivers/servo: reimplement with high level interface

[maribu]

Contribution description

The previous servo driver didn't provide any benefit over using PWM directly, as users controlled the servo in terms of PWM duty cycles. This changes the interface to provide a high level interface that abstracts the gory PWM details.

In addition, a SAUL layer and auto-initialization is provided.

Note that this driver's user of XFA and an array index as device descriptor, as well as decoupling the auto-initialization from the use of SAUL, is different to the way other sensor and actuator drivers are written.

Testing procedure

The test application provides a simple shell command that exposes the API, as well as the SAUL wrapper of the API.

> saul
2022-08-02 22:12:31,826 # saul
2022-08-02 22:12:31,827 # ID	Class		Name
2022-08-02 22:12:31,830 # #0	ACT_SWITCH	LD1(green)
2022-08-02 22:12:31,832 # #1	ACT_SWITCH	LD2(blue)
2022-08-02 22:12:31,834 # #2	ACT_SWITCH	LD3(red)
2022-08-02 22:12:31,837 # #3	SENSE_BTN	B1(User button)
2022-08-02 22:12:31,838 # #4	ACT_SERVO	servo
> saul write 4 0
2022-08-02 22:12:41,443 # saul write 4 0
2022-08-02 22:12:41,445 # Writing to device #4 - servo
2022-08-02 22:12:41,447 # Data:	             0 
2022-08-02 22:12:41,450 # [servo] setting 0 to 2949 (0 / 255)
2022-08-02 22:12:41,453 # data successfully written to device #4
> saul write 4 256
2022-08-02 22:12:45,343 # saul write 4 256
2022-08-02 22:12:45,346 # Writing to device #4 - servo
2022-08-02 22:12:45,347 # Data:	           256 
2022-08-02 22:12:45,351 # [servo] setting 0 to 6865 (255 / 255)
2022-08-02 22:12:45,354 # data successfully written to device #4
> servo 0 127
2022-08-02 22:12:51,347 # servo 0 127
2022-08-02 22:12:51,350 # [servo] setting 0 to 4899 (127 / 255)
> 2022-08-02 22:17:44,252 # Exiting Pyterm

Each write resulted in the MG90S servo that I connected to move to the corresponding position.

Note that it is indeed possible for SAUL and calls to the native driver API to co-exists, differently to all other sensor and actuator drivers.

Issues/PRs references

This show cases an alternative approach on decoupling auto-initialization from the use of SAUL compared to https://github.com/RIOT-OS/RIOT/pull/11871

[maribu]

I updated the code to calculate the duty cycle at runtime, as the actual PWM frequency is not known on compile time. E.g. on the Arduino UNO the actual PWM frequency is 61 Hz (if I recall correctly). With that, the actual extension and the requested one differs by 22 % if using the nominal frequency instead of the actual one.

(Yes, this works fine on the Arduino UNO when rebased on top of https://github.com/RIOT-OS/RIOT/pull/18148 :))

[maribu]

I don't quite understand the reason for use of XFA - why not use the same pattern as the other drivers?

Likely I should have done the conversion as a follow up. Note that there are two things that are changed: First, it allocates device states and params as an arrays and uses an index has handle instead of a pointer. Second, it uses XFA to implement the arrays.

The use of arrays has the following advantages:

• It safes RAM, as the current pattern of copying the params stuff into the state is no longer needed. (The alternative would be to add a pointer from the state to params, which would increase the state size by one pointer and add one level of pointer traversal to accessing params.)
• And most importantly: It fixes the fundamentally broken sensor and actuator auto initialization.
  • Auto-initialization does happen if used with SAUL, but then there is no way to access the device state and use it without the SAUL API.
  • When not using SAUL, currently no way of auto-initialization is possible
  • Using indexes solves the issue of obtaining the device pointer when initialization happens in auto_init

The use of XFA has the advantage that a user can easily add additional devices. E.g. let's say board foo already has 3 LEDs exposed via SAUL. Now we add additional LEDs via a shield. With XFA, we can just extend the LED array without having to mess with the board LED config.

Likely I should update an existing driver, maybe the SHT1x, to the XFA approach, first.

[maribu]

I dropped the use an of index-based servo_t and of XFA for consistency. The servo shell command had to be removed as well, as co-existence with SAUL no longer is possible.

This can be added in a follow up. Let's fix the servo API first.

[riot-ci]

Murdock results

:heavy_check_mark: PASSED

6dc2a6059717abe4518e14d25e543d3804c2d580 drivers/servo: reimplement with high level interface

Success	Failures	Total	Runtime
6865	0	6865	13m:10s

Artifacts

• Documentation preview

[maribu]

I greatly changed this PR, as I had urgent need to use this with the nRF52840. That MCU however cannot create PWM anywhere near the 50 Hz to 100 Hz required (I think the biggest prescaler could device the clock down to something like 32.678 or so). So I made servo_pwm and servo_timer submodules that contain different implementations of the same user facing API. The servo_timer implementation uses periph_timer_periodic to basically implement a 50 Hz PWM signal in software. Hence, servo_pwm remains the preferred implementation for every board that can generate slow enough PWM signals.

I'll dismiss the review now in the light of the mayor changes I just pushed.

[maribu]

I forgot the KConfig integration and added it now.

[benpicco]

I forgot the KConfig integration and added it now.

pretty much impossible to get that right on the first try :wink:

[benpicco]

bors merge

[benpicco]

bors cancel

[bors[bot]]

Canceled.

[benpicco]

bors merge

[bors[bot]]

Build failed (retrying...):

• Murdock

[bors[bot]]

This PR was included in a batch that was canceled, it will be automatically retried

[bors[bot]]

Build failed:

• Murdock

[maribu]

bors retry

Now with a Makefile.ci, let's see what happens now.

[bors[bot]]

:clock1: Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

[maribu]

bors retry

[bors[bot]]

Build succeeded:

• Murdock
• static-tests
• tools-build-success

[maribu]

Thx :)