AllStarLink / asl3-asterisk

Building enviornment for the .deb packages of Asterisk LTS + ASL3/app_rpt.
GNU General Public License v2.0
2 stars 0 forks source link

Updating "rxboost" in simpleusb.conf not working as expected #23

Closed Allan-N closed 4 months ago

Allan-N commented 4 months ago

While working on the changes to template-ize the simpleusb.conf file I was testing to see that the settings could be updated. One of my tests was to update the "rxboost" setting. The template includes "rxboost = no" so after toggling the state and writing the file I should see "rxboost = yes". Happily, I see that the update has been applied. Now, I go to toggle the state back and would expect to see either the "rxboost = yes" removed from the node (since it now matches the value in the template) or see "rxboost = no" (matching the new toggled state). Unfortunately, I am still seeing "rxboost = yes" in the file.

To repro with a SimpleUSB node :

  1. check that the simpleusb.conf looks as expected
  2. use the tune menu to change "rxboost" from off to on ... and save the changes
  3. check the simpleusb.conf file to see that "rxboost" is now 1/yes/true
  4. use the tunemenu to change "rxboost" back to off ... and save the changes
  5. check the simpleusb.conf file. Did "rxboost" get changed back to 0/no/false?

FYI : you can also test these changes without using the tune menu (simpleusb-tune-menu) with the following Asterisk CLI commands :

To view "rxboost" (get current settings)
  sudo asterisk -rx "susb tune menu-support 0"

  Note: "rxboost" is the 4th number in the returned string

To change "rxboost"
  sudo asterisk -rx "susb tune menu-support m0"
  sudo asterisk -rx "susb tune menu-support m1"

To save settings
  sudo asterisk -rx "susb tune menu-support j"
Allan-N commented 4 months ago

There are also issues with adding a var/value if the setting is not currently found in the file. One would have hoped that "update" would include "add" (or "insert") if not present but that does not appear to be the case. This makes starting off with an empty node that inherits from a template a bit of a challenge.

KB4MDD commented 4 months ago

@Allan-N what sound card device are you using? If it is a cm119B, the channel driver overrides the user setting to true/yes/enabled in the code. If you change the setting on the menu or manually in the configuration file, it will be set to true/yes/enables in the code.

@tsawyer has run into this problem previously.

Allan-N commented 4 months ago

@Allan-N what sound card device are you using? If it is a cm119B, the channel driver overrides the user setting to true/yes/enabled in the code. If you change the setting on the menu or manually in the configuration file, it will be set to true/yes/enables in the code.

It's a "cheap" CM108 dongle (https://www.amazon.com/gp/product/B0B2RDWXKZ)

... and I have been focused on the simpleusb.conf content (the settings in the file) and not the behavior.

KB4MDD commented 4 months ago

This appears to be a problem with Asterisk and templates. I just pulled the latest master and tested on my system. I am not using templates. The value changes to True and False respectively using the tune menu and write command.

I have Asterisk 20.3.0 built by root @ repeater on a x86_64 running Linux on 2023-06-12 14:28:52 UTC

We will need @InterLinked1 to look at the config code and see if there is some issue in Asterisk.

I may be related AllStarLink/app_rpt#51

Allan-N commented 4 months ago

You pulled the latest, 20.3.0? Seems like that would be older than what we appear to have been pulling/building, 20.7.0.

InterLinked1 commented 4 months ago

@Allan-N Can you provide a minimally reproducible example for this issue? i.e. a full simpleusb.conf that triggers this issue. The description is not sufficient to put together a config.

InterLinked1 commented 4 months ago

I may be related AllStarLink/app_rpt#51

The issue raised in AllStarLink/app_rpt#51 has a fix patched in using PhreakScript, so that issue was closed. Your install has this patch, right?

Allan-N commented 4 months ago

The issue raised in https://github.com/AllStarLink/app_rpt/issues/51 has a fix patched in using PhreakScript, so that issue was closed. Your install has this patch, right?

When I was doing my testing I started with apt install asl3 to install the packages built by @jxmx. I then made my changes to the .conf files (and "chan_simpleusb.c"), used asl3-asterisk/build-tree to build asterisk+app_rpt (with my changes) and then used dpkg to install only the config .deb and modules .deb. I do not see the same PhreakScript patching being done in the build-tree script. Probably need @jxmx to comment on how the packages are being built.

Allan-N commented 4 months ago

@Allan-N Can you provide a minimally reproducible example for this issue? i.e. a full simpleusb.conf that triggers this issue. The description is not sufficient to put together a config.

  1. clone/checkout the "326-template-ize-simpleusb.conf" branch of app_rpt

  2. copy "configs/rpt/rpt.conf" and "configs/rpt/simpleusb.conf" to "/etc/asterisk"

  3. restart asterisk

  4. tail /etc/asterisk/simpleusb.conf and note that [1999] does not have an "rxboost" line

  5. exec :

    sudo asterisk -rx "susb tune menu-support m1"
    sudo asterisk -rx "susb tune menu-support j"
    tail /etc/asterisk/simpleusb.conf

    You should now see "rxboost = yes" (or "true")

  6. exec :

    sudo asterisk -rx "susb tune menu-support m0"
    sudo asterisk -rx "susb tune menu-support j"
    tail /etc/asterisk/simpleusb.conf

    Now, the "rxboost" line should have been removed ... or you should now see "rxboost = no" (or "false")

Also, after going back and forth between "m1", "j", "m0", "j", "m1", "j" I ended up with the .conf file having extra "rxboost = yes" lines.

Allan-N commented 4 months ago

I may be related https://github.com/AllStarLink/app_rpt/issues/51

That fix looks to be related to ast_variable_retrieve(). With the tune code using ast_variable_update() the patch may not help.

InterLinked1 commented 4 months ago

I may be related AllStarLink/app_rpt#51

That fix looks to be related to ast_variable_retrieve(). With the tune code using ast_variable_update() the patch may not help.

Actually, it appears to be possibly related in that it's the same kind of issue, just sort of the opposite, writing instead of reading.

I didn't test this extensively, but this patch seems to address some of the issues raised here, curious if you find this works better for you than the existing code:

diff --git a/main/config.c b/main/config.c
index e7ac4149a6..e81d3afced 100644
--- a/main/config.c
+++ b/main/config.c
@@ -1533,13 +1533,19 @@ int ast_variable_delete(struct ast_category *category, const char *variable, con
 int ast_variable_update(struct ast_category *category, const char *variable,
                                                const char *value, const char *match, unsigned int object)
 {
-       struct ast_variable *cur, *prev=NULL, *newer=NULL;
+       struct ast_variable *cur, *prev=NULL, *newer=NULL, *matchvar = NULL;

        for (cur = category->root; cur; prev = cur, cur = cur->next) {
                if (strcasecmp(cur->name, variable) ||
                        (!ast_strlen_zero(match) && strcasecmp(cur->value, match)))
                        continue;
+               matchvar = cur;
+       }

+       for (cur = category->root; cur; prev = cur, cur = cur->next) {
+               if (cur != matchvar) {
+                       continue;
+               }
                if (!(newer = ast_variable_new(variable, value, cur->file)))
                        return -1;
Allan-N commented 4 months ago

Thank you for the changes. I will build, test, and report back in a few days.

Allan-N commented 4 months ago

Earlier, I wrote :

... I do not see the same PhreakScript patching being done in the build-tree script. Probably need @jxmx to comment on how the packages are being built.

Your config.c patch WAS applied (it's included in .../asl3-asterisk/debian/patches) :-)

Now to test the "writing" patch

Allan-N commented 4 months ago

I didn't test this extensively, but this patch seems to address some of the issues raised here, curious if you find this works better for you than the existing code:

The patch works GREAT :-) Now, when enabling "rxboost" we see the variable updated in the .conf file ("rxboost = yes") and when disabled the line is removed (the correct behavior as it now matches the template).

Allan-N commented 4 months ago

Moved this issue from [app_rpt] since the fix needs to be made to the Asterisk "ast_config*" APIs.

Will apply a "patch" here.

Allan-N commented 3 months ago

Filed : https://github.com/InterLinked1/phreakscript/issues/37