Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UART sensors cleanup #182

Merged
merged 10 commits into from
Jul 13, 2023
Merged

UART sensors cleanup #182

merged 10 commits into from
Jul 13, 2023

Conversation

laurensvalk
Copy link
Member

@laurensvalk laurensvalk commented Jul 10, 2023

This implements a start to pybricks/support#1140:

  • This is a start to separating the device connection manager and the uart device code.
  • Device synchronization is more likely to work in one go
  • Disconnecting is handled more quickly because we can drop various timeouts
  • Reduces code size on Move Hub by about 350 bytes

I have intentionally avoided much change to the dcm and uartdev code in this commit to make the change easier to process. We could do a few more cleanups though:

  • Fix mixed use of uartdev, data, port_data name for the main data structure
  • Fix names of the various protothreads: data->pt is not the same as data_pt, which can make it harder to follow.
  • Now that the dcm and uartdev run separately from start to end, each can be a proper protothread that reads from start to finish, instead of needing an external loop with a function call that causes it to run to the next yield. This will work the same but it could make the code easier to follow, especially since we omit the PT_SCHEDULE macro in a few places when we don't need the return value.

@laurensvalk laurensvalk changed the title Uart process UART sensors cleanup Jul 10, 2023
@coveralls
Copy link

coveralls commented Jul 10, 2023

Coverage Status

coverage: 55.999% (-0.1%) from 56.11% when pulling ef4cd2c on uart-process into 391d184 on master.

@laurensvalk
Copy link
Member Author

We could save code size on move hub and have faster start up on SPIKE hubs if we make this a configurable option and only enable it on the hubs that need it.

Originally posted by @dlech in #175 (comment)

@laurensvalk laurensvalk force-pushed the uart-process branch 2 times, most recently from c5df748 to 736e8bd Compare July 11, 2023 08:18
lib/pbio/drv/legodev/legodev_pup.c Outdated Show resolved Hide resolved
lib/pbio/drv/legodev/legodev_pup.c Outdated Show resolved Hide resolved
lib/pbio/drv/legodev/legodev_pup_uart.h Show resolved Hide resolved
lib/pbio/drv/legodev/legodev_test.c Show resolved Hide resolved
lib/pbio/drv/uart/uart.h Outdated Show resolved Hide resolved
lib/pbio/drv/legodev/legodev_pup.c Outdated Show resolved Hide resolved
lib/pbio/drv/legodev/legodev_pup.c Show resolved Hide resolved
lib/pbio/drv/legodev/legodev_pup.c Show resolved Hide resolved
lib/pbio/drv/legodev/legodev_pup.c Show resolved Hide resolved
lib/pbio/drv/legodev/legodev_pup_uart.c Outdated Show resolved Hide resolved
This way we know it is initialized when pbdrv_init completes.
The device connection manager and the uart device driver never need to
run at the same time, so drive them from a single process.

This reduces complexity and code size, and also reduces the time needed
to handle devices disconnecting since there are no longer multiple
timeouts to wait on.

It also separates the pup_uart code further so it can run on other
devices like EV3 which don't have the same connection manager.
- There were two different variables both named prev_type_id used for different things.
- Now uses one child protothread for the dcm and the uart sensor since they run at different times.
- Moved where the affirmative ids are kept.
There were mixes of port_data, data, and dev for the same types, which were not related to the actual data variables. Things like data->pt where very different from data_pt.

All pbdrv_legodev_pup_uart_dev_t are local to this file, so call them ludev for short.
Instead of using numerous goto's, we can return an error like we do everywhere in pbio. In this case bind it to the device for easy access.
This makes the main uart thread cleaner. Instead of resetting one thread with PT_INIT midway from another thread, organize them as follows:

- spawn synchronization, exit on error
- run send + receive in parallel until no data

This in turn can be spawned from the higher level process on platforms that support device detection, or just called indefinitely on platforms where uart mode is always active.
It was placed in legodev_pup to avoid the need for an extra process. But since it isn't enabled on Move Hub anyway, it makes more sense to place it where it belongs.
@laurensvalk laurensvalk merged commit ef4cd2c into master Jul 13, 2023
@laurensvalk laurensvalk deleted the uart-process branch July 13, 2023 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants