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

Automatically calculate attribute table size #176

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

petekubiak
Copy link
Collaborator

Closes #173.

This PR implements automatic sizing of the attribute table when using the gatt_server macro.
The number of attributes for each service is calculated in the gatt_service macro, and this is then summed up by the gatt_server macro to determine the size of the table.
The option to override this and manually set the attribute table size still exists.
I have also renamed the attribute macro argument to attribute_table_size as this more accurately reflects the value being set.

Topic for discussion

I have implemented a static assertion to throw a compile-time error if the table size is overridden with a value too small to hold the calculated number of attributes. The advantage of this is of course that the user will be notified before flashing their device, meaning that they do not need to be monitoring the device's trace to see the issue.
However, I cannot use formatted strings in a "constant assertion", so the compile-time error looks like this:

error[E0080]: evaluation of constant value failed
  --> src/ble_bas_peripheral.rs:22:1
   |
22 | #[gatt_server(attribute_table_size = 9)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'Specified attribute table size is insufficient. Please increase attribute_table_size or remove the argument entirely to allow automatic sizing of the attribute table.', src/ble_bas_peripheral.rs:22:1
   |
   = note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `core::assert` (in Nightly builds, run with -Z macro-backtrace for more info)

The alternative would be to use a normal assert! which would be evaluated at run-time. This would allow a formatted message of something like:

====================== PANIC ======================
panicked at /home/pkubiak/repos/trouble/examples/apps/src/ble_bas_peripheral.rs:22:1:
Specified attribute table size of 9 is insufficient. A table size of at least 10 is required for the current GATT server configuration.

but of course this would only be visible in the device's trace.

I personally lean toward the static assertion, but I'd be keen to hear other opinions on this!

@petekubiak petekubiak added the enhancement New feature or request label Nov 22, 2024
@petekubiak petekubiak self-assigned this Nov 22, 2024
@petekubiak
Copy link
Collaborator Author

/test

let buffer = meta.value().map_err(|_| Error::custom("attribute_data_size msut be followed by `= [size]`. e.g. attribute_data_size = 32".to_string()))?;
self.attribute_data_size = Some(buffer.parse()?);
"attribute_table_size" => {
let buffer = meta.value().map_err(|_| Error::custom("attribute_table_size msut be followed by `= [size]`. e.g. attribute_table_size = 32".to_string()))?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo there from older code "msut"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good eye

@jamessizeland
Copy link
Collaborator

Beautiful, and very nice code comments.

@petekubiak
Copy link
Collaborator Author

/test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhance gatt_server macro to automatically size the attribute table based on the number of defined attributes
2 participants