Convert bluetooth to MSGs with severity levels #90
Conversation
Signed-off-by: Mimoja <[email protected]>
There was a problem hiding this comment.
Great work!
I went cross-eyed trying to keep it all in perspective. I've got a few different opinions, but they're opinions.
I think that no matter what we do, we'll find reasons to adjust in the future. Getting these trimmed down enough so that normal users can run logging all the time at a moderate level (probably NOTICE and up) will, I think, help us all.
| @@ -117,7 +114,7 @@ proc skale_timer_start {} { | |||
| } | |||
|
|
|||
| if {[ifexists ::sinstance($::de1(suuid_skale))] == ""} { | |||
| msg "Skale not connected, cannot start timer" | |||
| msg -WARNING "Skale not connected, cannot start timer" | |||
There was a problem hiding this comment.
My first instinct was "these get called even when the device isn't connected". Rational thought overtook that quickly with "the test should happen before they are called."
I agree, just noting my thoughts
| @@ -383,7 +380,7 @@ proc hiroia_parse_response { value } { | |||
| } | |||
| ::device::scale::process_weight_update [expr $weight / 10.0] ;# $event_time | |||
| } else { | |||
| error "weight non exist" | |||
| msg -ERROR "weight non exist" | |||
There was a problem hiding this comment.
"Hiroia Jimmy Scale weight update did not provide weight"
| @@ -573,7 +568,7 @@ proc decentscale_enable_notifications {} { | |||
| } | |||
|
|
|||
| if {[ifexists ::sinstance($::de1(suuid_decentscale))] == ""} { | |||
| msg "decent scale not connected, cannot enable weight notifications" | |||
| msg -WARNING "decent scale not connected, cannot enable weight notifications" | |||
There was a problem hiding this comment.
Lots to check, but if consistently "Decent Scale" or whatever, makes grep easier to use.
Getting the logs trimmed down is such a huge step already. Thanks!
| @@ -1005,7 +975,7 @@ proc ble_connect_to_de1 {} { | |||
|
|
|||
| if {$::de1(device_handle) != "0"} { | |||
| catch { | |||
| msg "disconnecting from DE1" | |||
| msg -INFO "disconnecting from DE1" | |||
There was a problem hiding this comment.
For me, "high-level" connect/disconnect is interesting enough, especially for the support team, for NOTICE
| @@ -1021,13 +991,13 @@ proc ble_connect_to_de1 {} { | |||
| set ::de1_name "DE1" | |||
| if {[catch { | |||
| set ::currently_connecting_de1_handle [ble connect [string toupper $::settings(bluetooth_address)] de1_ble_handler false] | |||
| msg "Connecting to DE1 on $::settings(bluetooth_address)" | |||
| msg -INFO "Connecting to DE1 on $::settings(bluetooth_address)" | |||
There was a problem hiding this comment.
High-level connect/disconnect; NOTICE suggested
| msg "fw request to erase sent" | ||
| #msg "fw request to erase sent, now starting send" | ||
| #write_firmware_now | ||
| msg -INFO "fw request to erase sent" |
| firmware_upload_next | ||
| } else { | ||
| msg "MMR write ack: [string length $value] bytes: [convert_string_to_hex $value ] : $value : [array get arr2]" | ||
| msg -INFO "MMR write ack: [string length $value] bytes: [convert_string_to_hex $value ] : $value : [array get arr2]" |
| } else { | ||
| msg "Confirmed wrote to $cuuid of DE1: '$value'" | ||
| msg -INFO "Confirmed wrote to $cuuid of DE1: '$value'" |
| } | ||
| } elseif {$address == $::settings(scale_bluetooth_address)} { | ||
| msg "Confirmed wrote to $cuuid of $::settings(scale_type): '$value'" | ||
| msg -INFO "Confirmed wrote to $cuuid of $::settings(scale_type): '$value'" |
| } else { | ||
| msg "Confirmed wrote to $cuuid of unknown device: '$value'" | ||
| msg -INFO "Confirmed wrote to $cuuid of unknown device: '$value'" |
There was a problem hiding this comment.
WARNING -- code probably shouldn't reach this line and if it does, we should fix it
|
The set of questions around what level to report attempts to communicate with a device that isn't connected still have me shifting back and forth. I finally came down to the effort in fixing the code wasn't worth it, nor was alarming users with a WARNING as a result of the code's higher level logic not checking that the device was connected before making a request. |
First pass at assigning reasonable severituy codes for msg in bluetooth.tcl de1_comms.tcl gui.tcl utils.tcl Commented-out msg lines removed Converted one "gated" comms_msg to msg No work done to ensure that no binary data is being logged. Logging of add_de1_image removed as per discussion with John load_font completion retained at DEBUG level at this time These severities are likely to change in future commits, but need to get at least a first-cut at usability. Efforts of Mimoja are included in this commit, see #90 Signed-Off-By: Jeff Kletsky <[email protected]>
Signed-off-by: Mimoja [email protected]
@jeffsf can I ask you to have a look?