espidf added flush, clean task stop , commented out crashing debug log #362

vortex314 · 2024-02-29T16:42:14Z

trying to improve the serial espidf connectivity by adding timeouts in the session establishment .
Duplicate vTaskDelete removed
testing shows good results
flushing input and output makes recovery easier

eclipse-zenoh-bot · 2024-02-29T16:42:27Z

@vortex314 If this pull request contains a bugfix or a new feature, then please consider using Closes #ISSUE-NUMBER syntax to link it to an issue.

vortex314 · 2024-02-29T16:46:34Z

Test program can be found here : https://github.com/vortex314/zenoh-projects/tree/main/zenoh-serial

vortex314 · 2024-02-29T16:56:13Z

learning pull requests; Maybe shouldn´t have closed

cguimaraes · 2024-03-04T02:13:37Z

src/session/subscription.c

@@ -185,7 +185,7 @@ int8_t _z_trigger_subscriptions(_z_session_t *zn, const _z_keyexpr_t keyexpr, co
    zp_mutex_lock(&zn->_mutex_inner);
 #endif  // Z_FEATURE_MULTI_THREAD == 1

-    _Z_DEBUG("Resolving %d - %s on mapping 0x%x", keyexpr._id, keyexpr._suffix, _z_keyexpr_mapping_id(&keyexpr));
+//    _Z_DEBUG("Resolving %d - %s on mapping 0x%x", keyexpr._id, keyexpr._suffix, _z_keyexpr_mapping_id(&keyexpr));


Don't comment the debug lines.

I understand these pose issues when receiving something without a suffix, then I suggest you something like:

_Z_DEBUG("Resolving %d on mapping 0x%x", keyexpr._id, _z_keyexpr_mapping_id(&keyexpr)); _z_keyexpr_t key = __unsafe_z_get_expanded_key_from_key(zn, &keyexpr); if (key._suffix != NULL) { _Z_DEBUG("Triggering subs for %d - %s", key._id, key._suffix); _z_subscription_rc_list_t *subs = __unsafe_z_get_subscriptions_by_key(zn, _Z_RESOURCE_IS_LOCAL, key);

cguimaraes · 2024-03-04T02:15:29Z

src/system/espidf/network.c

+        int r = uart_read_bytes(sock._serial, &before_cobs[i], 1, 100);
+        if (r == 0) {
+            timeout_count++;
+            if (timeout_count > 10) {


Make this a configuration parameter, defined in the locator.
Ideally, a timer timeout instead of a counter timeout.

cguimaraes · 2024-03-04T02:16:31Z

src/system/espidf/network.c

+            if (timeout_count > 10) {
+                _Z_DEBUG("Timeout reading from serial");
+                zp_free(before_cobs);
+                return 0;


Avoid early returns to avoid breaking MISRA-C rules.

For MISRA-C rules we decided to relax rule 15.5 so early returns are welcomed if they're useful.

Is there a list of the rules that were decided to be relaxed?

At the very least 15.5 for early returns, and 17.7 in cases where the return value is truly meaningless.

cguimaraes · 2024-03-04T02:16:38Z

src/system/espidf/network.c

+        } else {
+            _Z_DEBUG("Error reading from serial");
+            zp_free(before_cobs);
+            return _Z_ERR_GENERIC;


Avoid early returns to avoid breaking MISRA-C rules.

…h lease and read task start top of 2 x 16 bytes - -DZ_FEATURE_SUBSCRIPTION=0 didn´t compile previously - used freeRtos in espidf - read now times out in 1 sec fixed

jean-roland · 2024-03-14T08:42:06Z

include/zenoh-pico/api/macros.h

@@ -279,8 +279,10 @@ template<> inline int8_t z_drop(z_owned_publisher_t* v) { return z_undeclare_pub
 template<> inline void z_drop(z_owned_keyexpr_t* v) { z_keyexpr_drop(v); }
 template<> inline void z_drop(z_owned_config_t* v) { z_config_drop(v); }
 template<> inline void z_drop(z_owned_scouting_config_t* v) { z_scouting_config_drop(v); }
+#if Z_FEATURE_SUBSCRIPTION==1


This one, I'd say we should either have all the feature token in the file or none.

Did you need this for a reason?

I seem to remember that otherwise it didn´t compile with Z_FEATURE_SUBSCRIPTION=0

lib/zenoh-pico/include/zenoh-pico/api/macros.h:282:72: error: 'z_undeclare_pull_subscriber' was not declared in this scope

jean-roland · 2024-03-14T08:44:44Z

src/api/api.c

@@ -746,15 +747,15 @@ int8_t z_publisher_put(const z_publisher_t pub, const uint8_t *payload, size_t l
                   opt.attachment
 #endif
    );
-
+#if Z_FEATURE_SUBSCRIPTION == 1


You shouldn't need this because _z_trigger_local_subscriptions is replaced by a dummy function if subscription function is off.... Suposedly

see comment above, it doesn´t compile. Feels like code was tested with this feature off.

No it's definitely tested with both in CI, I recall you use C++ to compile, so maybe this is the source of our compilation diverging behavior.

jean-roland · 2024-03-14T08:48:12Z

src/session/subscription.c

@@ -185,7 +185,7 @@ int8_t _z_trigger_subscriptions(_z_session_t *zn, const _z_keyexpr_t keyexpr, co
    zp_mutex_lock(&zn->_mutex_inner);
 #endif  // Z_FEATURE_MULTI_THREAD == 1

-    _Z_DEBUG("Resolving %d - %s on mapping 0x%x", keyexpr._id, keyexpr._suffix, _z_keyexpr_mapping_id(&keyexpr));
+//    _Z_DEBUG("Resolving %d - %s on mapping 0x%x", keyexpr._id, keyexpr._suffix, _z_keyexpr_mapping_id(&keyexpr));


I understand these pose issues when receiving something without a suffix, then I suggest you something like:

_Z_DEBUG("Resolving %d on mapping 0x%x", keyexpr._id, _z_keyexpr_mapping_id(&keyexpr)); _z_keyexpr_t key = __unsafe_z_get_expanded_key_from_key(zn, &keyexpr); if (key._suffix != NULL) { _Z_DEBUG("Triggering subs for %d - %s", key._id, key._suffix); _z_subscription_rc_list_t *subs = __unsafe_z_get_subscriptions_by_key(zn, _Z_RESOURCE_IS_LOCAL, key);

vortex314 · 2024-03-17T20:59:17Z

My attempts to merge fail, any suggestions. ? My Git kungfu is not strong enough.

lieven@pcpav2:~/workspace/zenoh-pico$ git push
To https://github.com/vortex314/zenoh-pico.git
 ! [rejected]          serial_retry -> serial_retry (non-fast-forward)
error: failed to push some refs to 'https://github.com/vortex314/zenoh-pico.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
lieven@pcpav2:~/workspace/zenoh-pico$ git status
On branch serial_retry
Your branch and 'origin/serial_retry' have diverged,
and have 14 and 3 different commits each, respectively.
  (use "git pull" to merge the remote branch into yours)

nothing to commit, working tree clean

vortex314 · 2024-03-19T18:43:26Z

too many changes already in main branch, try another pull request

espidf added flush, clean task stop , commented out crashing debug log

e1f4c49

vortex314 closed this Feb 29, 2024

vortex314 reopened this Feb 29, 2024

cguimaraes reviewed Mar 4, 2024

View reviewed changes

cguimaraes mentioned this pull request Mar 4, 2024

[Bug] Timeouts in Serial communication #140

Closed

vortex314 added 2 commits March 10, 2024 23:03

espidf serial working correctly , still a small memory leakage at eac…

e3752b9

…h lease and read task start top of 2 x 16 bytes - -DZ_FEATURE_SUBSCRIPTION=0 didn´t compile previously - used freeRtos in espidf - read now times out in 1 sec fixed

avoid dynamic allocation at each message received

8558865

jean-roland reviewed Mar 14, 2024

View reviewed changes

vortex314 closed this Mar 19, 2024

vortex314 deleted the serial_retry branch March 19, 2024 18:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

espidf added flush, clean task stop , commented out crashing debug log #362

espidf added flush, clean task stop , commented out crashing debug log #362

vortex314 commented Feb 29, 2024

eclipse-zenoh-bot commented Feb 29, 2024

vortex314 commented Feb 29, 2024

vortex314 commented Feb 29, 2024

cguimaraes Mar 4, 2024

jean-roland Mar 14, 2024

cguimaraes Mar 4, 2024

cguimaraes Mar 4, 2024

jean-roland Mar 4, 2024

cguimaraes Mar 4, 2024

jean-roland Mar 5, 2024

cguimaraes Mar 4, 2024

jean-roland Mar 14, 2024

vortex314 Mar 17, 2024

jean-roland Mar 14, 2024

vortex314 Mar 17, 2024

jean-roland Mar 18, 2024

jean-roland Mar 14, 2024

vortex314 commented Mar 17, 2024

vortex314 commented Mar 19, 2024

espidf added flush, clean task stop , commented out crashing debug log #362

espidf added flush, clean task stop , commented out crashing debug log #362

Conversation

vortex314 commented Feb 29, 2024

eclipse-zenoh-bot commented Feb 29, 2024

vortex314 commented Feb 29, 2024

vortex314 commented Feb 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vortex314 commented Mar 17, 2024

vortex314 commented Mar 19, 2024