<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    I've just merged your PR.<br>
    <br>
    `OvmsCommand.Register()` is well designed.<br>
    <br>
    On security: I don't think we actually need to allow adding user
    commands to system command groups at all.<br>
    <br>
    I'd opt for allowing only command root "usr", and optionally "x…"
    (vehicle specific namespaces), for user commands. Allowing "x…"
    should be a choice/config of the vehicle module.<br>
    <br>
    We could add a flag to the command definition and extend the
    registration accordingly to…<br>
    <br>
    <font face="monospace">    OvmsCommand* RegisterCommand(const char*
      name, const char* title,<br>
                                       OvmsCommandExecuteCallback_t
      execute = NULL,<br>
                                       const char *usage = "", int min =
      0, int max = 0, bool secure = true,<br>
                                       OvmsCommandValidateCallback_t
      validate = NULL,<br>
                                       <b>bool allow_user_commands =
        false</b>);</font><br>
    <br>
    Similar to the config param "usr" init, we then generally create an
    empty command root "usr" on system init with allow_user_commands =
    true.<br>
    <br>
    Vehicles can decide to create their full "x…" root, or some "x… usr"
    placeholder with that grant.<br>
    <br>
    What do you think?<br>
    <br>
    Regards,<br>
    Michael<br>
    <br>
    <br>
    <div class="moz-cite-prefix">Am 22.07.24 um 02:36 schrieb Michael
      Geddes via OvmsDev:<br>
    </div>
    <blockquote type="cite"
cite="mid:CAH0p7uJ0zseU5CmZ=64L1V3G6CLnr0WJ2q2Nm2USRxT=Lvinew@mail.gmail.com">
      <meta http-equiv="content-type" content="text/html; charset=UTF-8">
      <div dir="ltr">
        <div>I have tidied up the implementation significantly and I've
          fixed up all the life-time issues, as well as disallowing
          overriding existing commands.</div>
        <div>(Though you could still add a sub-command to a system
          command).  It now persists quite happily, doesn't crash on GC
          and functions as expected :). I've also implemented shut-down
          registering so it cleans up before getting to the destructor.</div>
        <div><br>
        </div>
        <div>It's now down to security and the question of whether we
          differentiate user plugin commands from system ones.</div>
        <div><br>
        </div>
        <div>//.ichael</div>
        <br>
        <div class="gmail_quote">
          <div dir="ltr" class="gmail_attr">On Sun, 21 Jul 2024 at
            08:34, Michael Geddes <<a
              href="mailto:frog@bunyip.wheelycreek.net"
              moz-do-not-send="true" class="moz-txt-link-freetext">frog@bunyip.wheelycreek.net</a>>
            wrote:<br>
          </div>
          <blockquote class="gmail_quote"
style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
            <div dir="ltr">Well since I've been doing some Duktape stuff
              lately, I recalled this loose end and have come up with a
              solution.
              <div><br>
              </div>
              <div>There were a few issues - please let me know if you
                have a problem with any of them.</div>
              <div><br>
              </div>
              <div>Firstly the Command registration map is using const
                char *  and we can't guarantee the lifetime of the
                duktape strings so I've moved them over to std::string.</div>
              <div>This applies to OvmsCommandMap itself as well as
                OvmsCommand - m_name, m_title and m_usage_template.</div>
              <div><br>
              </div>
              <div>I think the intention was to have something like this
                for the register command.. it was missing a parameter.
                It now looks like this:</div>
              <div>OvmsCommand.Register( function, parent, command,
                description, parameter_desc, minargs, maxargs).</div>
              <div>Missing maxargs will mean maxargs = minargs.</div>
              <div>Missing minargs will mean no parameters.</div>
              <div>The calling function will be passed 2 parameters: 
                the name of the command and an array of strings (the
                argv).</div>
              <div>For sub-commands, these will be separated by '/'. 
                 So  something like "me/too".</div>
              <div>I think this way is simple but effective.</div>
              <div>It would be good to add completion, but that can be
                added later.</div>
              <div><br>
              </div>
              <div>One of the things I wanted to talk about is what
                security protections we need to add to prevent a script
                from taking over an existing function! (or future
                function).</div>
              <div><br>
              </div>
              <div>When vim initially added user commands and functions
                it was a free-for-all but somebody soon realised there
                needed to be some restrictions to avoid issues..</div>
              <div>so what they did was force user function and commands
                to start with a capital letter.  I would like people's
                thoughts on whether we need to do something similar.</div>
              <div><br>
              </div>
              <div>It might be that we would be satisfied with failing
                if system commands were attempted to be overridden.
                Should we allow adding sub-commands to system commands?</div>
              <div>I can see that might be useful..   </div>
              <div><br>
              </div>
              <div>I have added a sample below of a working function:</div>
              <div><br>
              </div>
              <div>--8<----</div>
              <div><font face="monospace"><br>
                </font></div>
              <div><font face="monospace">mycommand = function(c, argv){<br>
                    print("Hello There: "+c+"\n");<br>
                    if (argv.length == 0)<br>
                      print("Simple\n");<br>
                    else<br>
                      print("Script: "+argv[0]+"\n")<br>
                    }<br>
                  OvmsCommand.Register(mycommand, "", "me", "My Script",
                  "[string]", 0, 3)</font><br>
              </div>
              <div><br>
              </div>
              <div><font face="monospace"><i>OVMS#</i> <b>me xx</b><br>
                  Hello There: me</font></div>
              <div><font face="monospace">Script: xx<br>
                  <i>OVMS# </i><b>me xx yy</b><br>
                  Hello There: me</font></div>
              <div><font face="monospace">Script: xx<br>
                  <i>OVMS#</i> <b>me xx yy</b><br>
                  Hello There: me<br>
                  Script: xx<br>
                  <i>OVMS#</i></font><br>
              </div>
              <div> </div>
            </div>
            <br>
            <div class="gmail_quote">
              <div dir="ltr" class="gmail_attr">On Sun, 23 Oct 2022 at
                22:15, Michael Balzer <<a
                  href="mailto:dexter@expeedo.de" target="_blank"
                  moz-do-not-send="true" class="moz-txt-link-freetext">dexter@expeedo.de</a>>
                wrote:<br>
              </div>
              <blockquote class="gmail_quote"
style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
                <div> That's an open end, implementing command
                  registration for Duktape.<br>
                  <br>
                  There's a thread on this… found it:<br>
                  <a
href="http://lists.openvehicles.com/pipermail/ovmsdev/2020-July/006952.html"
                    target="_blank" moz-do-not-send="true"
                    class="moz-txt-link-freetext">http://lists.openvehicles.com/pipermail/ovmsdev/2020-July/006952.html</a><br>
                  <br>
                  Regards,<br>
                  Michael<br>
                  <br>
                  <br>
                  <div>Am 23.10.22 um 15:38 schrieb Michael Geddes:<br>
                  </div>
                  <blockquote type="cite">
                    <div dir="ltr">
                      <div>There's an unused variable 'dcc' warning in
                        the below code.<br>
                      </div>
                      <div><br>
                      </div>
                      <div>It seems that the 'Perform the callback' is
                        not being 'performed!</div>
                      <div><br>
                      </div>
                      <div>Is this deliberate?</div>
                      <div><br>
                      </div>
                      <div><br>
                      </div>
                      <div>//.ichael</div>
                      <div><br>
                      </div>
                      <div><br>
                      </div>
                      <div><br>
                      </div>
                      <div>
                        <div
style="color:rgb(212,212,212);background-color:rgb(30,30,30);font-family:Consolas,"Courier New",monospace;font-size:14px;line-height:19px;white-space:pre-wrap"><div><span
                        style="color:rgb(86,156,214)">void</span> <span
                        style="color:rgb(220,220,170)">DukOvmsCommandRegisterRun</span>(<span
                        style="color:rgb(86,156,214)">int</span> <span
                        style="color:rgb(156,220,254)">verbosity</span>, <span
                        style="color:rgb(78,201,176)">OvmsWriter</span><span
                        style="color:rgb(86,156,214)">*</span> <span
                        style="color:rgb(156,220,254)">writer</span>, <span
                        style="color:rgb(78,201,176)">OvmsCommand</span><span
                        style="color:rgb(86,156,214)">*</span> <span
                        style="color:rgb(156,220,254)">cmd</span>, <span
                        style="color:rgb(86,156,214)">int</span> <span
                        style="color:rgb(156,220,254)">argc</span>, <span
                        style="color:rgb(86,156,214)">const</span> <span
                        style="color:rgb(86,156,214)">char*</span> <span
                        style="color:rgb(86,156,214)">const*</span> <span
                        style="color:rgb(156,220,254)">argv</span>)</div><div>  {</div><div>  <span
                        style="color:rgb(86,156,214)">ESP_LOGD</span>(<span
                        style="color:rgb(156,220,254)">TAG</span>, <span
                        style="color:rgb(206,145,120)">"DukOvmsCommandRegisterRun(</span><span
                        style="color:rgb(156,220,254)">%s</span><span
                        style="color:rgb(206,145,120)">)"</span>,<span
                        style="color:rgb(156,220,254)">cmd</span>-><span
                        style="color:rgb(220,220,170)">GetName</span>());</div>
<div>  <span style="color:rgb(86,156,214)">auto</span> <span
                        style="color:rgb(156,220,254)">it</span> = <span
                        style="color:rgb(156,220,254)">MyDuktape</span>.<span
                        style="color:rgb(156,220,254)">m_cmdmap</span>.<span
                        style="color:rgb(220,220,170)">find</span>(<span
                        style="color:rgb(156,220,254)">cmd</span>);</div><div>  <span
                        style="color:rgb(197,134,192)">if</span> (<span
                        style="color:rgb(156,220,254)">it</span> <span
                        style="color:rgb(220,220,170)">==</span> <span
                        style="color:rgb(156,220,254)">MyDuktape</span>.<span
                        style="color:rgb(156,220,254)">m_cmdmap</span>.<span
                        style="color:rgb(220,220,170)">end</span>())</div><div>    {</div><div>    <span
                        style="color:rgb(86,156,214)">ESP_LOGE</span>(<span
                        style="color:rgb(156,220,254)">TAG</span>, <span
                        style="color:rgb(206,145,120)">"Command '</span><span
                        style="color:rgb(156,220,254)">%s</span><span
                        style="color:rgb(206,145,120)">' cannot be found in registry"</span>,<span
                        style="color:rgb(156,220,254)">cmd</span>-><span
                        style="color:rgb(220,220,170)">GetName</span>());</div><div>    <span
                        style="color:rgb(197,134,192)">return</span>;</div><div>    }</div><div>  <span
                        style="color:rgb(197,134,192)">else</span></div><div>    {</div><div>    <span
                        style="color:rgb(78,201,176)">DuktapeConsoleCommand</span>* <span
                        style="color:rgb(156,220,254)">dcc</span> = <span
                        style="color:rgb(156,220,254)">it</span><span
                        style="color:rgb(220,220,170)">-></span><span
                        style="color:rgb(156,220,254)">second</span>;</div><div><span
                        style="color:rgb(106,153,85)">    // Perform the callback</span></div><div>    }</div><div>  }</div>
</div>
                      </div>
                    </div>
                    <br>
                    <fieldset></fieldset>
                    <pre>_______________________________________________
OvmsDev mailing list
<a href="mailto:OvmsDev@lists.openvehicles.com" target="_blank"
                    moz-do-not-send="true" class="moz-txt-link-freetext">OvmsDev@lists.openvehicles.com</a>
<a href="http://lists.openvehicles.com/mailman/listinfo/ovmsdev"
                    target="_blank" moz-do-not-send="true"
                    class="moz-txt-link-freetext">http://lists.openvehicles.com/mailman/listinfo/ovmsdev</a>
</pre>
                  </blockquote>
                  <br>
                  <pre cols="72">-- 
Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
Fon 02333 / 833 5735 * Handy 0176 / 206 989 26</pre>
                </div>
                _______________________________________________<br>
                OvmsDev mailing list<br>
                <a href="mailto:OvmsDev@lists.openvehicles.com"
                  target="_blank" moz-do-not-send="true"
                  class="moz-txt-link-freetext">OvmsDev@lists.openvehicles.com</a><br>
                <a
href="http://lists.openvehicles.com/mailman/listinfo/ovmsdev"
                  rel="noreferrer" target="_blank"
                  moz-do-not-send="true" class="moz-txt-link-freetext">http://lists.openvehicles.com/mailman/listinfo/ovmsdev</a><br>
              </blockquote>
            </div>
          </blockquote>
        </div>
      </div>
      <br>
      <fieldset class="moz-mime-attachment-header"></fieldset>
      <pre class="moz-quote-pre" wrap="">_______________________________________________
OvmsDev mailing list
<a class="moz-txt-link-abbreviated" href="mailto:OvmsDev@lists.openvehicles.com">OvmsDev@lists.openvehicles.com</a>
<a class="moz-txt-link-freetext" href="http://lists.openvehicles.com/mailman/listinfo/ovmsdev">http://lists.openvehicles.com/mailman/listinfo/ovmsdev</a>
</pre>
    </blockquote>
    <br>
    <pre class="moz-signature" cols="72">-- 
Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
Fon 02333 / 833 5735 * Handy 0176 / 206 989 26</pre>
  </body>
</html>