<html><head><meta http-equiv="content-type" content="text/html; charset=us-ascii"></head><body style="overflow-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;">I reviewed both, and they seem ok to me. We lose some debug functionality, but that tradeoff is outweighed by the benefit of a standard unpatched IDF. I doubt whether Espressif would accept most of these patched back into their mainstream, as they are of limited use for general users.<div><br></div><div>However, note that I am only really looking at this from our existing code and IDF point of view. The primary question being whether these changes would impact existing builds?</div><div><br></div><div>When we have a complete set of changes, I think we can then start to build in v5 IDF and really start testing on real devices then.</div><div><br></div><div>Regards, Mark<br><div><br><blockquote type="cite"><div>On 18 May 2023, at 6:05 AM, Ludovic LANGE <ll-ovmsdev@lange.nom.fr> wrote:</div><br class="Apple-interchange-newline"><div>
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<div><p>Hi List,</p><p>If possible, I'd like a peer-review / discussion of the following
PRs:</p>
<ul>
<li><a moz-do-not-send="true" href="https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/pull/901">ESP-IDF
v4+: we do not have uxMutexesHeld any more #901</a><br>
<blockquote type="cite">
<div>
<div class="edit-comment-hide">
<div class="comment-body markdown-body js-comment-body soft-wrap css-overflow-wrap-anywhere user-select-contain d-block"><p dir="auto">In ESP-IDF 3.3.x builds of OVMS, we're
using a fork of ESP-IDF. In this fork, FreeRTOS has
been patched with <a class="commit-link" data-hovercard-type="commit" data-hovercard-url="https://github.com/openvehicles/esp-idf/commit/95e43fc2c4360f8126a0985bf037a293bebe3767/hovercard" href="https://github.com/openvehicles/esp-idf/commit/95e43fc2c4360f8126a0985bf037a293bebe3767">openvehicles/esp-idf@<tt>95e43fc</tt></a>
to add a <code class="notranslate">uxMutexesHeld</code>
field in <code class="notranslate">TaskStatus_t</code>.</p><p dir="auto">However, in the ESP-IDF >= 4 build
we're using mainstream ESP-IDF (for the moment), which
does not include this field.</p><p dir="auto">So we adapt the display to handle both
cases.</p>
</div>
</div>
</div>
</blockquote>
<br>
</li>
<li><a moz-do-not-send="true" href="https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/pull/902">ESP-IDF4+
: crash handler and boot status #902</a><br>
<blockquote type="cite">
<div>
<div class="edit-comment-hide">
<div class="comment-body markdown-body js-comment-body soft-wrap css-overflow-wrap-anywhere user-select-contain d-block">
<ul dir="auto">
<li>error handling rework (<code class="notranslate">xt_set_error_handler_callback</code>
is only available in our ESP-IDF 3.3.4 fork.)</li>
<li><code class="notranslate">esp_task_wdt_get_trigger_tasknames()</code>
not available (<strong>feature removed</strong> from
ESP-IDF4+ builds) TODO</li>
<li>first steps to support other archs than XTensa in
<code class="notranslate">boot_data.crash_data</code></li>
</ul>
</div>
</div>
</div>
</blockquote>
<br>
</li>
</ul><p>These are the <b>last 2 PRs</b> before being able to build with
any of these ESP-IDF versions : 3.3.4-ovms, 4.4.4, 5.0, 5.0.1,
branch-v5.0, branch-v5.1, latest - the last mile !</p><p>(One <a href="https://github.com/openvehicles/Open-Vehicle-Monitoring-System-3/pull/903">workaround
PR</a> is pending to be able to <b>run</b> one of these new
versions, preventing a crash at boot - but it needs a proper
analysis first)<br>
</p><p><br>
</p><div><br class="webkit-block-placeholder"></div><p>Also, with these 2 PRs, we will be able to enable automatic
builds on all these versions on master (using GitHub "Actions") -
you can have a preview of what I mean here :
<a class="moz-txt-link-freetext" href="https://github.com/llange/Open-Vehicle-Monitoring-System-3/actions">https://github.com/llange/Open-Vehicle-Monitoring-System-3/actions</a></p><p>As we're slowly entering a period of transition between ESP-IDF
3.3.4-ovms and a more recent one (5.0.1 ?), we will need a way to
ensure that the PRs / commits are OK across all these versions.
Automatic builds can be configured to build on all targeted
ESP-IDF versions, and can report their result back in the PRs, so
that we can trap potential compilation errors before merging.<br>
(Especially as in the newer ESP-IDF versions, more and more
warnings are treated as compilation errors)</p><p>I also prepared a check for documentation errors using the same
mechanism.</p><p><br>
</p>
Thanks in advance.
<p>Regards,<br>
</p>
<div id="grammalecte_menu_main_button_shadow_host" style="width:
0px; height: 0px;"></div>
</div>
_______________________________________________<br>OvmsDev mailing list<br>OvmsDev@lists.openvehicles.com<br>http://lists.openvehicles.com/mailman/listinfo/ovmsdev<br></div></blockquote></div><br></div></body></html>