Change default configuration / allow config via register API

[generationmake]

according to spec

[generationmake]

Hi @aentinger any proposals how to change node id if no node id was set?

[aentinger]

No. That's not supported for now (and will not be for some time).

[aentinger]

BTW. As discussed in another PRs thread the default value for a unset node id shall be zero.

[generationmake]

hi @aentinger could you please update this firmware to the latest features with eeprom file system and so on. Thanks

[aentinger]

Hi @generationmake :coffee: :wave:

I've just checked, the firmware already contains everything to have persistent storage and register API. Did you try and test it? If it doesn't work it could be because your Pico core runs a too old GCC version.

[aentinger]

This fixes #20. This fixes #21.

[aentinger]

Hi @generationmake :coffee: :wave:

All subscription should work again, the tricky part is to configure the correct CAN filters to let the subscription messages pass through the hardware filter.

Please test :bow: .

[generationmake]

Hi @aentinger I made a short test and found no errors or malfunction. Is there a possibility to also store the settings for update period in the eeprom?

[aentinger]

Is there a possibility to also store the settings for update period in the eeprom?

Done :+1:

[generationmake]

Hi @aentinger I implemented factory reset. But it seems like it is always performing a reset right after factory reset. Could you investigate this? Maybe it is a built-in function of yakut? Thanks

[generationmake]

Hi @aentinger I tried to make node.description writable, but it is not compiling. could you also investigate this?

[aentinger]

Hi @aentinger I tried to make node.description writable, but it is not compiling. could you also investigate this?

According to the CI build it compiles just fine. Are you sure about the "not-compiling"? What's the error?

[generationmake]

Hi. I get the following error message with Arduino 1.8.19 (sorry for the long post):

In file included from /home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/Node.hpp:21,
                 from /home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/107-Arduino-Cyphal.h:15,
                 from /home/bernhard/projects/OpenCyphalPicoBase-firmware/OpenCyphalPicoBase-firmware.ino:18:
/home/bernhard/.arduino15/packages/rp2040/tools/pqt-gcc/1.5.0-b-c7bab52/arm-none-eabi/include/c++/12.2.0/optional: In instantiation of 'class std::optional<char [25]>':
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_impl.hpp:299:39:   required from 'registry::RegisterPtr registry::Registry::expose(N&&, const Options&, T&) [with N = const char (&)[24]; T = char [25]; <template-parameter-1-3> = void; registry::RegisterPtr = std::unique_ptr<registry::Register, std::default_delete<registry::Register> >]'
/home/bernhard/projects/OpenCyphalPicoBase-firmware/OpenCyphalPicoBase-firmware.ino:222:84:   required from here
/home/bernhard/.arduino15/packages/rp2040/tools/pqt-gcc/1.5.0-b-c7bab52/arm-none-eabi/include/c++/12.2.0/optional:1024:9: error: function returning an array
 1024 |         value_or(_Up&& __u) const&
      |         ^~~~~~~~
/home/bernhard/.arduino15/packages/rp2040/tools/pqt-gcc/1.5.0-b-c7bab52/arm-none-eabi/include/c++/12.2.0/optional:1037:9: error: function returning an array
 1037 |         value_or(_Up&& __u) &&
      |         ^~~~~~~~
In file included from /home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/Node.hpp:31:
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_impl.hpp: In instantiation of 'registry::RegisterPtr registry::Registry::expose(N&&, const Options&, T&) [with N = const char (&)[24]; T = char [25]; <template-parameter-1-3> = void; registry::RegisterPtr = std::unique_ptr<registry::Register, std::default_delete<registry::Register> >]':
/home/bernhard/projects/OpenCyphalPicoBase-firmware/OpenCyphalPicoBase-firmware.ino:222:84:   required from here
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_impl.hpp:299:21: error: invalid array assignment
  299 |                 val = registry::get<T>(v).value();  // Guaranteed to be coercible by the protocol.
      |                 ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_base.hpp:10,
                 from /home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_impl.hpp:10:
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_value.hpp: In instantiation of 'std::optional<_Tp> registry::get(const Value&) [with T = char [25]; Value = uavcan::_register::Value_1_0]':
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_impl.hpp:299:39:   required from 'registry::RegisterPtr registry::Registry::expose(N&&, const Options&, T&) [with N = const char (&)[24]; T = char [25]; <template-parameter-1-3> = void; registry::RegisterPtr = std::unique_ptr<registry::Register, std::default_delete<registry::Register> >]'
/home/bernhard/projects/OpenCyphalPicoBase-firmware/OpenCyphalPicoBase-firmware.ino:222:84:   required from here
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_value.hpp:241:20: error: no matching function for call to 'get(const registry::Value&, char [25])'
  241 |     if (detail::get(src, out))
      |         ~~~~~~~~~~~^~~~~~~~~~
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_value.hpp:144:20: note: candidate: 'template<class T, unsigned int N> bool registry::detail::get(const registry::Value&, std::array<_Tp, _Nm>&)'
  144 | [[nodiscard]] bool get(const Value& src, std::array<T, N>& dst)
      |                    ^~~
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_value.hpp:144:20: note:   template argument deduction/substitution failed:
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_value.hpp:241:20: note:   mismatched types 'std::array<_Tp, _Nm>' and 'char [25]'
  241 |     if (detail::get(src, out))
      |         ~~~~~~~~~~~^~~~~~~~~~
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_value.hpp:154:20: note: candidate: 'template<class T, class> bool registry::detail::get(const registry::Value&, T&)'
  154 | [[nodiscard]] bool get(const Value& src, T& dst)
      |                    ^~~
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_value.hpp:154:20: note:   template argument deduction/substitution failed:
In file included from /home/bernhard/.arduino15/packages/rp2040/tools/pqt-gcc/1.5.0-b-c7bab52/arm-none-eabi/include/c++/12.2.0/bits/stl_pair.h:60,
                 from /home/bernhard/.arduino15/packages/rp2040/tools/pqt-gcc/1.5.0-b-c7bab52/arm-none-eabi/include/c++/12.2.0/bits/stl_algobase.h:64,
                 from /home/bernhard/.arduino15/packages/rp2040/tools/pqt-gcc/1.5.0-b-c7bab52/arm-none-eabi/include/c++/12.2.0/bits/specfun.h:45,
                 from /home/bernhard/.arduino15/packages/rp2040/tools/pqt-gcc/1.5.0-b-c7bab52/arm-none-eabi/include/c++/12.2.0/cmath:1935,
                 from /home/bernhard/.arduino15/packages/rp2040/tools/pqt-gcc/1.5.0-b-c7bab52/arm-none-eabi/include/c++/12.2.0/math.h:36,
                 from /home/bernhard/.arduino15/packages/rp2040/hardware/rp2040/3.1.0/cores/rp2040/api/../../../ArduinoCore-API/api/ArduinoAPI.h:47,
                 from /home/bernhard/.arduino15/packages/rp2040/hardware/rp2040/3.1.0/cores/rp2040/api/ArduinoAPI.h:2,
                 from /home/bernhard/.arduino15/packages/rp2040/hardware/rp2040/3.1.0/cores/rp2040/Arduino.h:28,
                 from /tmp/arduino_build_276719/sketch/OpenCyphalPicoBase-firmware.ino.cpp:1:
/home/bernhard/.arduino15/packages/rp2040/tools/pqt-gcc/1.5.0-b-c7bab52/arm-none-eabi/include/c++/12.2.0/type_traits: In substitution of 'template<bool _Cond, class _Tp> using enable_if_t = typename std::enable_if::type [with bool _Cond = false; _Tp = void]':
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_value.hpp:153:23:   required from 'std::optional<_Tp> registry::get(const Value&) [with T = char [25]; Value = uavcan::_register::Value_1_0]'
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_impl.hpp:299:39:   required from 'registry::RegisterPtr registry::Registry::expose(N&&, const Options&, T&) [with N = const char (&)[24]; T = char [25]; <template-parameter-1-3> = void; registry::RegisterPtr = std::unique_ptr<registry::Register, std::default_delete<registry::Register> >]'
/home/bernhard/projects/OpenCyphalPicoBase-firmware/OpenCyphalPicoBase-firmware.ino:222:84:   required from here
/home/bernhard/.arduino15/packages/rp2040/tools/pqt-gcc/1.5.0-b-c7bab52/arm-none-eabi/include/c++/12.2.0/type_traits:2614:11: error: no type named 'type' in 'struct std::enable_if<false, void>'
 2614 |     using enable_if_t = typename enable_if<_Cond, _Tp>::type;
      |           ^~~~~~~~~~~
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_value.hpp: In instantiation of 'std::optional<_Tp> registry::get(const Value&) [with T = char [25]; Value = uavcan::_register::Value_1_0]':
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_impl.hpp:299:39:   required from 'registry::RegisterPtr registry::Registry::expose(N&&, const Options&, T&) [with N = const char (&)[24]; T = char [25]; <template-parameter-1-3> = void; registry::RegisterPtr = std::unique_ptr<registry::Register, std::default_delete<registry::Register> >]'
/home/bernhard/projects/OpenCyphalPicoBase-firmware/OpenCyphalPicoBase-firmware.ino:222:84:   required from here
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_value.hpp:164:27: note: candidate: 'bool registry::detail::get(const registry::Value&, std::string_view&)' (near match)
  164 | [[nodiscard]] inline bool get(const Value& src, std::string_view& dst)
      |                           ^~~
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_value.hpp:164:27: note:   conversion of argument 2 would be ill-formed:
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_value.hpp:241:26: error: cannot bind non-const lvalue reference of type 'std::string_view&' {aka 'std::basic_string_view<char>&'} to an rvalue of type 'std::string_view' {aka 'std::basic_string_view<char>'}
  241 |     if (detail::get(src, out))
      |                          ^~~
In file included from /home/bernhard/.arduino15/packages/rp2040/tools/pqt-gcc/1.5.0-b-c7bab52/arm-none-eabi/include/c++/12.2.0/bits/basic_string.h:47,
                 from /home/bernhard/.arduino15/packages/rp2040/tools/pqt-gcc/1.5.0-b-c7bab52/arm-none-eabi/include/c++/12.2.0/string:53,
                 from /home/bernhard/.arduino15/packages/rp2040/tools/pqt-gcc/1.5.0-b-c7bab52/arm-none-eabi/include/c++/12.2.0/bitset:47,
                 from /home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/types/uavcan/node/port/ServiceIDList_1_0.hpp:47,
                 from /home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/nodeinfo/../../types/uavcan/node/port/List_1_0.hpp:47,
                 from /home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/nodeinfo/../../DSDL_Types.h.impl:12,
                 from /home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/nodeinfo/../../DSDL_Types.h:20,
                 from /home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/nodeinfo/NodeInfoBase.hpp:17,
                 from /home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/Node.hpp:30:
/home/bernhard/.arduino15/packages/rp2040/tools/pqt-gcc/1.5.0-b-c7bab52/arm-none-eabi/include/c++/12.2.0/string_view:133:7: note:   after user-defined conversion: 'constexpr std::basic_string_view<_CharT, _Traits>::basic_string_view(const _CharT*) [with _CharT = char; _Traits = std::char_traits<char>]'
  133 |       basic_string_view(const _CharT* __str) noexcept
      |       ^~~~~~~~~~~~~~~~~
/home/bernhard/Arduino/libraries/107-Arduino-Cyphal/src/util/registry/registry_value.hpp:243:16: error: could not convert 'out' from 'char [25]' to 'std::optional<char [25]>'
  243 |         return out;
      |                ^~~
      |                |
      |                char [25]

[aentinger]

CI didn't bark because those code parts are blocked out due to a too old compiler for the CI build. I'll look into this 👍

[generationmake]

Hi @aentinger I added the watchdog reset to the factory reset. The board is not resetting any more. Yakut still runs into a timeout. It seems like the erasing of the eeprom takes around 1.2 seconds. the default timeout of yakut is 1.0 seconds. So this issue is OK for me.

[aentinger]

Hi @generationmake :coffee: :wave:

Firmware is compiling now :+1: .

Please test:

• Read node description
• Set node description to a string shorter than/larger than/equal to "OpenCyphalPicoBase" and report the results here.

Furthermore did you test:

• Set Servo PWM via yakut? Did it work?
• Set Digital Output via yakut? Did it work?

It would be worth documenting those yakut commands in the README.

[generationmake]

Hi @aentinger I have tested it and I can confirm it compiles. But it is not working. I cannot set or change the description. like this:

bernhard@notebookR60P-22:~$ y r 0 cyphal.node.description
OpenCyphalPicoBase              
bernhard@notebookR60P-22:~$ y r 0 cyphal.node.description "test"
"0\x1D\0 "                      
bernhard@notebookR60P-22:~$ y r 0 cyphal.node.description
"0\x1D\0 "     

the behavior is always the same, regardless of the text.

[generationmake]

Hi @aentinger I tested servo and output and everything works as expected! I also updated the readme.

[aentinger]

Good Morning @pavel-kirienko :coffee: :wave:

We've got a question concerning on how to use the register API for a character array.

I was pretty sure that this and this would not work.

Which statements would work?

Cheers, Alex

[pavel-kirienko]

You need something that owns a mutable string. Perhaps this static String class (see archive): https://github.com/OpenCyphal/CETL/issues/14. Then you go like:

String<256> value;
registry.expose(name, {.persistent = true}, value);

You can use std::string as well if you have it.

On a side note, I notice that there's an unhealthy amount of copy-pasting in the linked code. It almost seems like you need some abstraction for Cyphal ports.

[aentinger]

Well it didn't work with std::string, see error above.

No thanks on CETL for now, you've linked to an unmerged PR and I'm tired of being guinea pig number one :grin: :stuck_out_tongue_closed_eyes:

[pavel-kirienko]

Well it didn't work with std::string

I don't see std::string there. There appears to be string_view and a plain character array, these won't work.

No thanks on CETL for now, you've linked to an unmerged PR

This is not a PR, the link is to an archive attached there. However, as you can use std::string, you should just use that as it is simpler.

[aentinger]

Following at @pavel-kirienko's suggestion (thank you :bow: ) I've replaced the std::string_view with std::string. @generationmake can you please test?

[generationmake]

@aentinger This is not compiling...

[aentinger]

This is not compiling...

Too bad, it isn't. I just did an edit online, as I'm currently sick and did not compile it.

@pavel-kirienko, with std::string I'm getting some interesting template error, see out.txt.

Do you see a easy fix for this? If not, I'm calling it quits, you simply can't change the node description (for now).

[pavel-kirienko]

Yes, we'll need to add a new get overload here https://github.com/107-systems/107-Arduino-Cyphal/blob/4d4e4bfca51dbe2bbba487de82e2bf1e705d231c/src/util/registry/registry_value.hpp#L173 as follows:

[[nodiscard]] inline bool get(const Value& src, std::string& dst)
{
    if (const auto* const str = src.get_string_if())
    {
        dst = std::string{reinterpret_cast<const char*>(str->value.data()), str->value.size()};
        return true;
    }
    return false;
}

[aentinger]

Hi @generationmake :coffee: :wave:

Can you please test this PR in combination with https://github.com/107-systems/107-Arduino-Cyphal/pull/238 ?

Now it support for std::string as register value should work :+1: :pray:

[generationmake]

Hi @aentinger A first test was successful. I will do further tests but it looks good now.

[generationmake]

how long could this string be?

[aentinger]

how long could this string be?

256 bytes I shall think (see here). Not sure if that includes the zero-terminator (it should not, so 256 readable characters).

[aentinger]

New release of 107-Arduino-Cyphal (v3.2.1) containing the necessary fixes for std::string as registry parameter.

[generationmake]

it seems like yakut does not allow strings longer than 256:

bernhard@notebookR60P-22:~$ y r 11 cyphal.node.description "12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890"
OverflowError: Python int too large to convert to C long

[generationmake]

Hi @aentinger I have performed several tests using a checklist and I have found no errors or issues. So I assume this firmware is fine and we are ready to merge.

[aentinger]

it seems like yakut does not allow strings longer than 256:

Well, this certainly makes sense, given that Yakut knows the DSDL.

[aentinger]

So I assume this firmware is fine and we are ready to merge.

Yes, good to merge! Go for it!