Skip to content

Close the param threads properly if close is called. #535

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.

Sign up for GitHub

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

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

Close the param threads properly if close is called. #535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d582c8c
Close the param threads properly if close is called.
ToveRumar Apr 3, 2025
4e3e6fe
Wait to start param thread for when the link is open.
ToveRumar Apr 4, 2025
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
6 changes: 3 additions & 3 deletions cflib/crazyflie/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def __init__(self, link=None, ro_cache=None, rw_cache=None):
self.extpos = Extpos(self)
self.log = Log(self)
self.console = Console(self)
self.param = Param(self)
self.param = None
Copy link
Preview

Copilot AI Apr 24, 2025

Choose a reason for hiding this comment

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

Setting self.param to None in the initializer may lead to potential null reference errors if other parts of the code access param before open_link is called. Consider adding a guard or clearly documenting this initialization sequence.

Suggested change
self.param = None
self.param = Param(self)

Copilot uses AI. Check for mistakes.

self.mem = Memory(self)
self.platform = PlatformService(self)
self.appchannel = Appchannel(self)
Expand All @@ -137,8 +137,6 @@ def __init__(self, link=None, ro_cache=None, rw_cache=None):

self.connected_ts = None

self.param.all_updated.add_callback(self._all_parameters_updated)

# Connect callbacks to logger
self.disconnected.add_callback(
lambda uri: logger.info('Callback->Disconnected from [%s]', uri))
Expand Down Expand Up @@ -244,6 +242,8 @@ def open_link(self, link_uri):
self.connection_requested.call(link_uri)
self.state = State.INITIALIZED
self.link_uri = link_uri
self.param = Param(self)
self.param.all_updated.add_callback(self._all_parameters_updated)
try:
self.link = cflib.crtp.get_link_driver(
link_uri, self.link_statistics.radio_link_statistics_callback, self._link_error_cb)
Expand Down
10 changes: 9 additions & 1 deletion cflib/crazyflie/param.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,8 @@ def _close(self):

# Then force an unlock of the mutex if we are waiting for a packet
# we didn't get back due to a disconnect for example.
self._should_close = True
self.request_queue.put(None)
try:
self._lock.release()
except RuntimeError:
Expand All @@ -584,6 +586,8 @@ def _close(self):
def run(self):
while not self._should_close:
pk = self.request_queue.get() # Wait for request update
if pk is None: # Put none to close the thread
continue
self._lock.acquire()
if self._cf.link:
self._req_param = struct.unpack('<H', pk.data[1:3])[0]
Expand Down Expand Up @@ -616,9 +620,11 @@ def close(self):
self.request_queue.get(block=False)
except Empty:
pass

self._should_close = True
self.request_queue.put(None)
# Then force an unlock of the mutex if we are waiting for a packet
# we didn't get back due to a disconnect for example.

try:
self.wait_lock.release()
except RuntimeError:
Expand Down Expand Up @@ -677,6 +683,8 @@ def request_param_update(self, var_id):
def run(self):
while not self._should_close:
pk = self.request_queue.get() # Wait for request update
if pk is None: # Put none to close the thread
continue
self.wait_lock.acquire()
if self.cf.link:
if self._useV2:
Expand Down