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

bluetooth: mesh: update heartbeat use of k_work API #33148

Merged
merged 2 commits into from
Mar 10, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 38 additions & 30 deletions subsys/bluetooth/mesh/heartbeat.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,25 @@ struct hb_pub_val {

static struct bt_mesh_hb_pub pub;
static struct bt_mesh_hb_sub sub;
static struct k_delayed_work sub_timer;
static struct k_delayed_work pub_timer;
static struct k_work_delayable sub_timer;
static struct k_work_delayable pub_timer;

static int64_t sub_remaining(void)
{
if (sub.dst == BT_MESH_ADDR_UNASSIGNED) {
return 0U;
}

return k_delayed_work_remaining_get(&sub_timer) / MSEC_PER_SEC;
uint32_t rem_ms = k_ticks_to_ms_floor32(
k_work_delayable_remaining_get(&sub_timer));

return rem_ms / MSEC_PER_SEC;
}

static void hb_publish_end_cb(int err, void *cb_data)
{
if (pub.period && pub.count > 1) {
k_delayed_work_submit(&pub_timer, K_SECONDS(pub.period));
k_work_reschedule(&pub_timer, K_SECONDS(pub.period));
}

if (pub.count != 0xffff) {
Expand Down Expand Up @@ -149,17 +152,18 @@ static void hb_publish(struct k_work *work)

BT_DBG("hb_pub.count: %u", pub.count);

/* Fast exit if disabled or expired */
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is a subtle behavior change here: for publish where the count had expired publish could detect a lost subnet and change state. Assuming that is not required behavior I've moved the fast-exit for expired/disabled before the subnet check.

Copy link
Contributor

Choose a reason for hiding this comment

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

This works. Looks like the publish disabled decision point isn't handled consistently, but I'll address this in a separate PR after this goes in.

if (pub.period == 0U || pub.count == 0U) {
return;
}

sub = bt_mesh_subnet_get(pub.net_idx);
if (!sub) {
BT_ERR("No matching subnet for idx 0x%02x", pub.net_idx);
pub.dst = BT_MESH_ADDR_UNASSIGNED;
return;
}

if (pub.count == 0U) {
return;
}

err = heartbeat_send(&publish_cb, NULL);
if (err) {
hb_publish_end_cb(err, NULL);
Expand All @@ -186,8 +190,8 @@ int bt_mesh_hb_recv(struct bt_mesh_net_rx *rx, struct net_buf_simple *buf)
return 0;
}

if (!k_delayed_work_pending(&sub_timer)) {
BT_DBG("Heartbeat subscription period expired");
if (!k_work_delayable_is_pending(&sub_timer)) {
BT_DBG("Heartbeat subscription inactive");
return 0;
}

Expand Down Expand Up @@ -216,7 +220,11 @@ static void pub_disable(void)
pub.ttl = 0U;
pub.period = 0U;

k_delayed_work_cancel(&pub_timer);
/* Try to cancel, but it's OK if this still runs (or is
* running) as the handler will be a no-op if it hasn't
* already checked period for being non-zero.
*/
(void)k_work_cancel_delayable(&pub_timer);
}

uint8_t bt_mesh_hb_pub_set(struct bt_mesh_hb_pub *new_pub)
Expand Down Expand Up @@ -248,12 +256,11 @@ uint8_t bt_mesh_hb_pub_set(struct bt_mesh_hb_pub *new_pub)
/* The first Heartbeat message shall be published as soon as possible
* after the Heartbeat Publication Period state has been configured for
* periodic publishing.
*
* If the new configuration disables publishing this flushes
* the work item.
*/
if (pub.period && pub.count) {
k_delayed_work_submit(&pub_timer, K_NO_WAIT);
} else {
k_delayed_work_cancel(&pub_timer);
}
k_work_reschedule(&pub_timer, K_NO_WAIT);

if (IS_ENABLED(CONFIG_BT_SETTINGS)) {
bt_mesh_settings_store_schedule(
Expand Down Expand Up @@ -295,28 +302,26 @@ uint8_t bt_mesh_hb_sub_set(uint16_t src, uint16_t dst, uint32_t period)
sub.min_hops = 0U;
sub.max_hops = 0U;
sub.count = 0U;
sub.period = sub.period - sub_remaining();
if (!k_delayed_work_cancel(&sub_timer)) {
notify_sub_end();
}
sub.period = 0U;
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll need to follow this line up with a PTS run, I think. I couldn't find any places in the test spec where the old behavior was required, but it could be a quirk in the PTS implementation.

Won't let this block this PR though.
cc @michal-narajowski

Copy link
Collaborator

Choose a reason for hiding this comment

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

I ran HBP and HBS tests with this PR and they are passing so this looks good to me.

} else if (period) {
sub.src = src;
sub.dst = dst;
sub.min_hops = BT_MESH_TTL_MAX;
sub.max_hops = 0U;
sub.count = 0U;
sub.period = period;
k_delayed_work_submit(&sub_timer, K_SECONDS(period));
} else {
/* Clearing the period should stop heartbeat subscription
* without clearing the parameters, so we can still read them.
*/
sub.period = sub.period - sub_remaining();
if (!k_delayed_work_cancel(&sub_timer)) {
notify_sub_end();
}
sub.period = 0U;
}

/* Start the timer, which notifies immediately if the new
* configuration disables the subscription.
*/
k_work_reschedule(&sub_timer, K_SECONDS(sub.period));

return STATUS_SUCCESS;
}

Expand Down Expand Up @@ -347,28 +352,31 @@ void bt_mesh_hb_feature_changed(uint16_t features)
void bt_mesh_hb_init(void)
{
pub.net_idx = BT_MESH_KEY_UNUSED;
k_delayed_work_init(&pub_timer, hb_publish);
k_delayed_work_init(&sub_timer, sub_end);
k_work_init_delayable(&pub_timer, hb_publish);
k_work_init_delayable(&sub_timer, sub_end);
}

void bt_mesh_hb_start(void)
{
if (pub.count && pub.period) {
BT_DBG("Starting heartbeat publication");
k_delayed_work_submit(&pub_timer, K_NO_WAIT);
k_work_reschedule(&pub_timer, K_NO_WAIT);
}
}

void bt_mesh_hb_suspend(void)
{
k_delayed_work_cancel(&pub_timer);
/* Best-effort suspend. This cannot guarantee that an
* in-progress publish will not complete.
*/
(void)k_work_cancel_delayable(&pub_timer);
}

void bt_mesh_hb_resume(void)
{
if (pub.period && pub.count) {
BT_DBG("Starting heartbeat publication");
k_delayed_work_submit(&pub_timer, K_NO_WAIT);
k_work_reschedule(&pub_timer, K_NO_WAIT);
}
}

Expand Down