Status not printed

[dkerr64]

Describe the bug I have print_format = "Xml" in my toml file but nothing is printing out to stdout. I tried Human as well and it also fails. This used to work so something seems to have changed.

To Reproduce config file...

[[cameras]]
name = "camera"
username = "admin"
password = "XXXX"
uid = "952XXXXX"
address = "192.168.12.XX"
discovery = "local"
stream = "mainStream"
push_notifications = false
print_format = "Xml"

Command...

~/github/neolink/target/release/neolink image --use-stream --config=/home/david/neolink.toml --file-path=/home/david/image.jpeg camera

Expected behavior It should report status including battery level and temperature.

Output

david@neolink:~$ ~/github/neolink/target/release/neolink image --use-stream --config=/home/david/neolink.toml --file-path=/home/david/image.jpeg driveway
[2024-10-16T21:29:35Z INFO  neolink] Neolink 0.6.3-rc.3 (unknown commit) release
[2024-10-16T21:29:35Z INFO  neolink::utils] driveway: Connecting to camera at Address: 192.168.12.XX, UID: 952XXXXX
[2024-10-16T21:29:35Z INFO  neolink_core::bc_protocol] driveway: Trying TCP discovery
[2024-10-16T21:29:36Z INFO  neolink_core::bc_protocol] driveway: Trying local discovery
[2024-10-16T21:29:38Z INFO  neolink_core::bc_protocol] driveway: Local discovery success 952XXXXX at 192.168.12.XX:33072
[2024-10-16T21:29:38Z INFO  neolink::utils] driveway: Logging in
[2024-10-16T21:29:38Z INFO  neolink::utils] driveway: Connected and logged in
[2024-10-16T21:29:40Z INFO  neolink::common::camthread] driveway: Camera time is already set: 2024-10-16 17:29:43.0 +05:00:00
[2024-10-16T21:29:42Z INFO  neolink::common::neocam] driveway: Model Reolink Argus 3 Pro
[2024-10-16T21:29:42Z INFO  neolink::common::neocam] driveway: Firmware Version v3.0.0.4019_24090555
[2024-10-16T21:29:44Z INFO  neolink::image::gst] appsrc name=thesource ! h265parse ! decodebin ! jpegenc snapshot=TRUE
                    ! filesink location=/home/David/image.jpeg

[QuantumEntangledAndy]

Have you tried the actual battery command? The printing of battery status is for the long running commands like rtsp and mqtt not the one off ones like image. The idea being to not pollute the stdout with extra information not related to the command asked to perform.

[QuantumEntangledAndy]

For short commands like image your expected to run battery command for status when you want it

[dkerr64]

Okay, so that is definitely a change as it used to report that with images. And it appears that I cannot use both battery and image commands at the same time.

This feels like a bad change to me. My use case is that I am capturing an image from my cameras every minute and superimpose on top of the image the current battery/temperature, like below image. Now the problem is that if it is separate command, each takes about 15 seconds to complete... which doubles the queries to the cameras. And surely that is going to be bad for battery life?

My cameras are mounted to light posts, and get power to recharge each night for about 6 hours, then have to run 18 hours on battery... but if temperatures go below zero, then the battery does not charge. So I like to monitor this on my surveillance page (which updates once a minute).

Can we either put it back to always report the status, or add an option to other commands?

Thanks

[dkerr64]

Have you tried the actual battery command? The printing of battery status is for the long running commands like rtsp and mqtt not the one off ones like image. The idea being to not pollute the stdout with extra information not related to the command asked to perform.

The config file allows for print_format = "None" why isn't that good enough to suppress the output?

[QuantumEntangledAndy]

I believe it was in #54 (which was quite some time ago) where it was first changed.

There are a few commands that print their information as xml into the stdout. It did not seem good practice to pollute any command's output with battery status in this way since it's unpredictable how it will change the output (before or after for example). I consider the old behaviour an unintended effect since I never planned for both of them to be printed I prefer proper separation of concerns for the commands.

Perhaps if you want the same behaviour as before again you could consider adding it as an optional command line argument? I'll accept a PR along those lines to make it a proper controllable feature.

[dkerr64]

Is this the commit that removed it, or should I be looking elsewhere? https://github.com/QuantumEntangledAndy/neolink/commit/17e8ab0f37b34b8a42fbf726e8dfe22223515b65

[dkerr64]

Is this the commit that removed it, or should I be looking elsewhere? 17e8ab0

Answering my own question... yes. If I add back in the deletes from that commit, I get the battery status back. Now to think about how I make it an option.

[QuantumEntangledAndy]

Try looking at src/cmdline.rs Specifically an option like

    #[arg(short, long, global = true, value_parser = PathBuf::from_str)]
    pub config: Option<PathBuf>,

maybe something like

    #[arg(short, long, global = true, action)]
    pub battery_info: bool,

[dkerr64]

Thanks for the pointer. I'm not familiar with rust so I ran into problems with adding a cmdline flag but got it working by adding a option to config.toml...

#[serde(default = "default_false", alias = "battery", alias = "battery_info")]
pub(crate) print_battery_info: bool,

and

if options.print_battery_info {
   if let Err(e) = me.monitor_battery(options.aux_printing).await {
      warn!("Could not monitor battery: {:?}", e);
   }
}

The problem I ran into with adding a command line flag is that the global cmdline variable is declared in main.rs as let opt = Opt::parse(); so it is local in scope to main() and is not passed down to any other module. How would I make the global cmdline options available in bc_protocol.rs? can I declare opt in global scope?

I'm not familiar enough with Rush so apologies if this is a really basic.

Thanks.

[QuantumEntangledAndy]

I see. Well the more direct method. Would be to add the option on to the commands flags for example src/pir/cmdline.rs for each one that wants it. Which likely gives us fine control over which command need it.