search

AllStarLink / app_rpt

Refactoring and upgrade of AllStarLink's app_rpt, etc.
8 stars 6 forks source link

chan_simpleusb: susb tune write #291

Closed tsawyer closed 6 months ago

tsawyer commented 6 months ago

First attempt to use the combined tune settings provided...

Bench*CLI> susb tune save
Saved radio tuning settings to simpleusb_tune_2502.conf
[2024-03-12 14:40:23.635] ERROR[118030]: config.c:2720 is_writable: Unable to write to directory /etc/asterisk/custom (No such file or directory)
[2024-03-12 14:40:23.635] WARNING[118030]: chan_simpleusb.c:3116 tune_write: Failed to save config

As a test I created a custom directory in /etc/asterisk and did another susb tune save and it wrote:

cat /etc/asterisk/custom/simpleusb.conf

;!
;! Automatically generated configuration file
;! Filename: simpleusb.conf (/etc/asterisk/custom/simpleusb.conf)
;! Generator: chan_simpleusb
;! Creation Date: Tue Mar 12 14:45:51 2024
;!
Allan-N commented 6 months ago

I noticed this when I was working on the PHP menu code. It's due to the #tryinclude at the end of the simpleusb.conf file and is noted in https://github.com/AllStarLink/asl3-menu/blob/develop/php-backend/UpdateConfig-notes.md.

InterLinked1 commented 6 months ago

@tsawyer Doesn't address the custom thing, which I think is from your config it sounds like, but there is a bug here that I've fixed in the linked patch.

tsawyer commented 6 months ago

My simpleusb.conf had a #include ... rather than a #tryinclude .... Copied from the configs/rpt dir to fix.

tsawyer commented 6 months ago

@InterLinked1 The issue-291 branch still wants the custom dir to exist, complains when it dose not, and writes the same incorrect simpleusb.conf mentioned above when it dose exists.

Its strange chan_sampleusb is insisting on the existence of the custom directory. None of the other configs seem to do that.

/etc/asterisk/simpleusb.conf did update correctly with this branch.

InterLinked1 commented 6 months ago

@InterLinked1 The issue-291 branch still wants the custom dir to exist, complains when it dose not, and writes the same incorrect simpleusb.conf mentioned above when it dose exists.

Its strange chan_sampleusb is insisting on the existence of the custom directory. None of the other configs seem to do that.

I don't think it has anything to do with the module, but rather what @Allan-N pointed out with your config. Does it happen if you remove that line?

Allan-N commented 6 months ago

@InterLinked1 Q? why does asterisk/AMI change the #tryinclude to #include when a conf file is updated? Intentional? or just a bug?

@tsawyer Q? do we "need" the #tryinclude in simpleusb.conf (and others)?

InterLinked1 commented 6 months ago

@InterLinked1 Q? why does asterisk/AMI change the #tryinclude to #include when a conf file is updated? Intentional? or just a bug?

That is a bug which I will look into.

@tsawyer Q? do we "need" the #tryinclude in simpleusb.conf (and others)?

That's what I'm thinking not... if the file doesn't exist, you don't need to attempt to include it.

I realize you want to keep it in the provided starter configs for people in case they have it, but if you don't have one, it wouldn't hurt to remove and see if that goes away.

Allan-N commented 6 months ago

My thoughts were that the #tryinclude was added as a way to guide folks to put their changes in the .../custom file ... and the main config would find the file if it exists with no ill effect if the file was missing. But, one could also say "if you are gonna customize the configuration then you're already making changes :-)"

Allan-N commented 6 months ago

Alternatively, we might thing about adding a "#tryinclude custom/simpleusb_.conf" directive to each node category with the hopes that folks won't mess with the file our tools will be manipulating. Kinda like saying "put your changes in that file and leave this file alone".

InterLinked1 commented 6 months ago

Alternatively, we might thing about adding a "#tryinclude custom/simpleusb_.conf" directive to each node category with the hopes that folks won't mess with the file our tools will be manipulating. Kinda like saying "put your changes in that file and leave this file alone".

From the perspective of the config tools, it doesn't really matter whether configuration is in the main file or included in another, it shouldn't have a problem.

An approach that I use for a module that updates the configs like this is to put the auto-managed file in another file that's included in the main file (so sort of the opposite approach, where custom stuff is in the main file). The config code doesn't really care, it's able to work with it either way.

tsawyer commented 6 months ago

@tsawyer Q? do we "need" the #tryinclude in simpleusb.conf (and others)?

@InterLinked1 No, we don't need them. Others seem to like them so we should probably leave them.

I removed the #tryinclude and susb tune save seemed to work well.

tsawyer commented 6 months ago

I found weird behaviors when #tryinclude and the custom directory exists.

  1. susb tune save changes the #tryinclude to #include, the try is removed.
  2. Asterisk won't start and coredumps with #include and no custom directory
[2024-03-13 07:45:37.297]  Loading chan_simpleusb.so.
[2024-03-13 07:45:37.481] ERROR[33048]: config.c:2127 process_text_line: The file 'custom/simpleusb.conf' was listed as a #include but it does not exist.
[1]    33048 segmentation fault  asterisk -c

core-asterisk-2024-03-13T14-53-18Z-full.txt

tsawyer commented 6 months ago

Are the two save messages expected behavior for susb tune save ?

Bench*CLI> susb tune save
Saved radio tuning settings to simpleusb.conf
[2024-03-13 08:02:04.451]   == Saving '/etc/asterisk/simpleusb.conf'
[2024-03-13 08:02:04.452]   == Saving '/etc/asterisk/simpleusb.conf': saved
InterLinked1 commented 6 months ago

Are the two save messages expected behavior for susb tune save ?

Bench*CLI> susb tune save Saved radio tuning settings to simpleusb.conf [2024-03-13 08:02:04.451] == Saving '/etc/asterisk/simpleusb.conf' [2024-03-13 08:02:04.452] == Saving '/etc/asterisk/simpleusb.conf': saved

Yes, you've asked this before, there are always 2 messages, for every config.

InterLinked1 commented 6 months ago

I found weird behaviors when #tryinclude and the custom directory exists.

  1. susb tune save changes the #tryinclude to #include, the try is removed.
  2. Asterisk won't start and coredumps with #include and no custom directory

[2024-03-13 07:45:37.297] Loading chan_simpleusb.so. [2024-03-13 07:45:37.481] ERROR[33048]: config.c:2127 process_text_line: The file 'custom/simpleusb.conf' was listed as a #include but it does not exist. [1] 33048 segmentation fault asterisk -c core-asterisk-2024-03-13T14-53-18Z-full.txt

Thanks Tim! I updated the linked PR to hopefully address this as well, could you try this again and see if it still segfaults?

tsawyer commented 6 months ago

@InterLinked1 Why did this happen? Any recommendation to fix?

root@Bench ➜  app_rpt git:(issue-291) git pull
remote: Enumerating objects: 5, done.
remote: Counting objects: 100% (5/5), done.
remote: Total 5 (delta 4), reused 5 (delta 4), pack-reused 0
Unpacking objects: 100% (5/5), 812 bytes | 23.00 KiB/s, done.
From https://github.com/InterLinked1/app_rpt
 + d00240a...2ee59e4 issue-291  -> origin/issue-291  (forced update)
hint: You have divergent branches and need to specify how to reconcile them.
hint: You can do so by running one of the following commands sometime before
hint: your next pull:
hint:
hint:   git config pull.rebase false  # merge
hint:   git config pull.rebase true   # rebase
hint:   git config pull.ff only       # fast-forward only
hint:
hint: You can replace "git config" with "git config --global" to set a default
hint: preference for all repositories. You can also pass --rebase, --no-rebase,
hint: or --ff-only on the command line to override the configured default per
hint: invocation.
fatal: Need to specify how to reconcile divergent branches.
InterLinked1 commented 6 months ago

I amended my original commit and force pushed, which overwrote the history on that branch, so this is honestly expected.

The easiest way might be to delete the branch locally and then repull it from the remote. There's probably a better way but I'm not certain about it.

tsawyer commented 6 months ago

Working well now. No segfaults, no changing of the #tryinclude line.

I still question why when #tryinclude is set it writes a custom/simpleusb.conf containing only:

root@Bench ➜  asterisk cat custom/simpleusb.conf
;!
;! Automatically generated configuration file
;! Filename: simpleusb.conf (/etc/asterisk/custom/simpleusb.conf)
;! Generator: chan_simpleusb
;! Creation Date: Wed Mar 13 11:43:18 2024
;!
Allan-N commented 6 months ago

When #tryinclude was set and AMI updated did you get both the [new] custom/simpleusb.conf AND the #tryinclude was replaced with #include? If so, that's the bug!

tsawyer commented 6 months ago

The #tryinclude replacement with #include bug appears to be fixed.

The bug that is not fixed is with #tryinclude the AMI writes two files:

  1. /etc/asterisk/simpleusb.conf is updated properly.
  2. /etc/asterisk/custom/simpleusb.conf contains only 6 lines of comments (nothing else) as follows: 
    root@Bench ➜  asterisk cat custom/simpleusb.conf
;!
;! Automatically generated configuration file
;! Filename: simpleusb.conf (/etc/asterisk/custom/simpleusb.conf)
;! Generator: chan_simpleusb
;! Creation Date: Wed Mar 13 11:43:18 2024
;!