Skip to content
This repository has been archived by the owner on Feb 19, 2024. It is now read-only.

Implement ble_gatts_value_set() #195

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

dangu
Copy link
Contributor

@dangu dangu commented May 11, 2021

Implement ble_gatts_value_set(). A Python class BLEGattsValue() is also created to feed sd_ble_gatts_value_set()

A Python class BLEGattsValue() is also created to feed sd_ble_gatts_value_set()
Copy link
Contributor

@jornbh jornbh left a comment

Choose a reason for hiding this comment

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

These changes are missing tests. If you want to add them, you can follow the format of the tests in the tests/ folder.

@@ -1466,6 +1466,24 @@ def to_c(self):
char_md.p_sccd_md = self.sccd_md.to_c()
return char_md

class BLEGattsValue(object):
def __init__(self, value=[0], offset=0):
Copy link
Contributor

Choose a reason for hiding this comment

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

Having lists can as the default parameters to functions can often be risky, since any change the list will persist between function calls. It is often safer to do:

f(arg=None):
    if arg == None:
        arg = [0]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! That was new to me. Updating with None as default parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, about writing tests: I'm not sure how to write a test involving ble_gatts_value_set() as I need a conn_handle and a handle for it?

jornbh and others added 3 commits June 30, 2022 15:09
@dangu dangu force-pushed the feature/ble_gatts_value_set branch from 5ff8ff2 to ae009cf Compare June 30, 2022 21:16
Copy link
Contributor

@jornbh jornbh left a comment

Choose a reason for hiding this comment

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

Since creating a gatt-database will be so much work, just for a single test, I think it should be fine to skip the test for this one as long as the code looks sound enough.

Comment on lines +1471 to +1473
def __init__(self, value=None, offset=0):
if value == None:
value = [0]
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason for why you want the default value to be a list with 0 elements?

Also, do you support the functionality for when p_value is set to NULL(https://infocenter.nordicsemi.com/topic/com.nordic.infocenter.s132.api.v5.0.0/structble__gatts__value__t.html)?

Copy link
Contributor

Choose a reason for hiding this comment

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

One version of the underlying c-function can be found at
ble_gatts_value_set

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants