You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2021/10/28 13:42:59 UTC
[GitHub] [mynewt-nimble] fjmolinas opened a new pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
fjmolinas opened a new pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065
This PR exposes `ble_gap_adv_active_instance`, it's sometimes useful to check if there is an active advertisement on an instance before stopping it or changing some parameter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] fjmolinas commented on a change in pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
fjmolinas commented on a change in pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065#discussion_r739082705
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -1294,14 +1294,12 @@ ble_gap_master_in_progress(void)
#endif
}
-#if NIMBLE_BLE_ADVERTISE || NIMBLE_BLE_CONNECT
-static int
+int
ble_gap_adv_active_instance(uint8_t instance)
{
/* Assume read is atomic; mutex not necessary. */
Review comment:
Maybe another return code is more appropriate when the instance is invalid?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] sjanc commented on a change in pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065#discussion_r739076479
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -1294,14 +1294,12 @@ ble_gap_master_in_progress(void)
#endif
}
-#if NIMBLE_BLE_ADVERTISE || NIMBLE_BLE_CONNECT
-static int
+int
ble_gap_adv_active_instance(uint8_t instance)
{
/* Assume read is atomic; mutex not necessary. */
Review comment:
I meant add new function ble_gap_ext_adv_active(uint8_t instance) that will internally use ble_gap_adv_active_instance()
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] fjmolinas commented on a change in pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
fjmolinas commented on a change in pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065#discussion_r739316059
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -1303,6 +1303,16 @@ ble_gap_adv_active_instance(uint8_t instance)
}
#endif
+#if MYNEWT_VAL(BLE_EXT_ADV)
+int ble_gap_ext_adv_active(uint8_t instance)
+{
+ if (instance >= BLE_ADV_INSTANCES) {
+ return -BLE_HS_EINVAL;
Review comment:
Addressed as you said, it might be confusing to have 1 be a valid output and 3 an error though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] sjanc commented on a change in pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065#discussion_r739307105
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -1303,6 +1303,16 @@ ble_gap_adv_active_instance(uint8_t instance)
}
#endif
+#if MYNEWT_VAL(BLE_EXT_ADV)
+int ble_gap_ext_adv_active(uint8_t instance)
+{
+ if (instance >= BLE_ADV_INSTANCES) {
+ return -BLE_HS_EINVAL;
Review comment:
this should be return BLE_HS_EINVAL (no minus)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] fjmolinas commented on a change in pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
fjmolinas commented on a change in pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065#discussion_r739258551
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -1294,14 +1294,12 @@ ble_gap_master_in_progress(void)
#endif
}
-#if NIMBLE_BLE_ADVERTISE || NIMBLE_BLE_CONNECT
-static int
+int
ble_gap_adv_active_instance(uint8_t instance)
{
/* Assume read is atomic; mutex not necessary. */
Review comment:
squashed directly
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] fjmolinas commented on pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
fjmolinas commented on pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065#issuecomment-957294791
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] sjanc commented on a change in pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065#discussion_r740922602
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -1303,6 +1303,16 @@ ble_gap_adv_active_instance(uint8_t instance)
}
#endif
+#if MYNEWT_VAL(BLE_EXT_ADV)
+int ble_gap_ext_adv_active(uint8_t instance)
+{
+ if (instance >= BLE_ADV_INSTANCES) {
+ return -BLE_HS_EINVAL;
Review comment:
this is actually a good point, so lets just return 0 if instance is invalid... (sorry for jumping back and forth on this)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] sjanc commented on a change in pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065#discussion_r740922602
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -1303,6 +1303,16 @@ ble_gap_adv_active_instance(uint8_t instance)
}
#endif
+#if MYNEWT_VAL(BLE_EXT_ADV)
+int ble_gap_ext_adv_active(uint8_t instance)
+{
+ if (instance >= BLE_ADV_INSTANCES) {
+ return -BLE_HS_EINVAL;
Review comment:
this is actually a good point, so lets just return 0 if instance is invalid... (sorry for jumping back and forth on this)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] fjmolinas commented on a change in pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
fjmolinas commented on a change in pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065#discussion_r739313740
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -1303,6 +1303,16 @@ ble_gap_adv_active_instance(uint8_t instance)
}
#endif
+#if MYNEWT_VAL(BLE_EXT_ADV)
+int ble_gap_ext_adv_active(uint8_t instance)
+{
+ if (instance >= BLE_ADV_INSTANCES) {
+ return -BLE_HS_EINVAL;
Review comment:
I must be dyslexic, you had said it before and I somehow read `note the minus`., sorry for this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] fjmolinas commented on pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
fjmolinas commented on pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065#issuecomment-957294791
Is this one ok now @sjanc?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] sjanc commented on a change in pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065#discussion_r739056094
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -1294,14 +1294,12 @@ ble_gap_master_in_progress(void)
#endif
}
-#if NIMBLE_BLE_ADVERTISE || NIMBLE_BLE_CONNECT
-static int
+int
ble_gap_adv_active_instance(uint8_t instance)
{
/* Assume read is atomic; mutex not necessary. */
Review comment:
There is already ble_gap_adv_active() for legacy API.
So I'd add new API ble_gap_ext_adv_active() (under EXT_ADV ifdef) that would wrap this (and check if instance is valid)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] fjmolinas commented on pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
fjmolinas commented on pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065#issuecomment-957294791
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] sjanc commented on a change in pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065#discussion_r739223903
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -1294,14 +1294,12 @@ ble_gap_master_in_progress(void)
#endif
}
-#if NIMBLE_BLE_ADVERTISE || NIMBLE_BLE_CONNECT
-static int
+int
ble_gap_adv_active_instance(uint8_t instance)
{
/* Assume read is atomic; mutex not necessary. */
Review comment:
other functions typically use BLE_HS_EINVAL for such cases, lets keep it consistent (note no minus)
other than that looks OK to me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] sjanc merged pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
sjanc merged pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] sjanc commented on a change in pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065#discussion_r740922602
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -1303,6 +1303,16 @@ ble_gap_adv_active_instance(uint8_t instance)
}
#endif
+#if MYNEWT_VAL(BLE_EXT_ADV)
+int ble_gap_ext_adv_active(uint8_t instance)
+{
+ if (instance >= BLE_ADV_INSTANCES) {
+ return -BLE_HS_EINVAL;
Review comment:
this is actually a good point, so lets just return 0 if instance is invalid... (sorry for jumping back and forth on this)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] sjanc merged pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
sjanc merged pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] sjanc merged pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
sjanc merged pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] fjmolinas commented on a change in pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
fjmolinas commented on a change in pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065#discussion_r739067366
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -1294,14 +1294,12 @@ ble_gap_master_in_progress(void)
#endif
}
-#if NIMBLE_BLE_ADVERTISE || NIMBLE_BLE_CONNECT
-static int
+int
ble_gap_adv_active_instance(uint8_t instance)
{
/* Assume read is atomic; mutex not necessary. */
Review comment:
I had done that initially, but the function is used in legacy api code, just with instance=0 and so travis complained. I could wrap the `static` qualifier based on `EXT_ADV` ifdef. Would that be ok?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] fjmolinas commented on a change in pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
fjmolinas commented on a change in pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065#discussion_r739082073
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -1294,14 +1294,12 @@ ble_gap_master_in_progress(void)
#endif
}
-#if NIMBLE_BLE_ADVERTISE || NIMBLE_BLE_CONNECT
-static int
+int
ble_gap_adv_active_instance(uint8_t instance)
{
/* Assume read is atomic; mutex not necessary. */
Review comment:
Pushed, is it what you had in mind?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-nimble] fjmolinas commented on pull request #1065: nimble/host/include/host/ble_gap: expose ble_gap_adv_active_instance
Posted by GitBox <gi...@apache.org>.
fjmolinas commented on pull request #1065:
URL: https://github.com/apache/mynewt-nimble/pull/1065#issuecomment-957823149
Thank you for the review! @sjanc
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org