Skip to content
This repository has been archived by the owner on Jan 14, 2023. It is now read-only.

first pass at saving history out to file on exit #28

Merged
merged 5 commits into from
Aug 19, 2020

Conversation

jacobrosenthal
Copy link
Contributor

feel free to bike shed

@Yatekii
Copy link
Member

Yatekii commented Jul 17, 2020

Thanks for providing this implementation.
I think this feature is direly needed!

What I would change tho is this: Instead of just enabling logs and have cargo.embed clutter your entire source dir with logs, we could make it such that one can provide an RTT log directory so the config flag would be of type Option<Path>.
And then we could structure the logs on a per run basis. So could make a structure like this:

rtt_log_dir
    binary1_target1_timestamp1
    binary1_target1_timestamp2
    binary1_target2_timestamp1
    binary2_target1_timestamp1

etc.

What do you think? :)

@jacobrosenthal
Copy link
Contributor Author

defaults path to ./logs
Im generating the timestamp in main and sending it into app::new so we get a single timestamp

So with my two channels Im gettng
logs
magic_wand_2020-07-20T15:18:25.458863350-07:00_0.txt
magic_wand_2020-07-20T15:18:25.458863350-07:00_1.txt

The rtt channel is kinda getting lost in the back of the date. Also I cant seem to find the target.. And im stealing the name from the args, not sure thats the best place, presumably its being parsed somewhere else?

@jacobrosenthal
Copy link
Contributor Author

Made channel more obvious
Could probably cut time down more but Ive got no opinion to what.
Not sure if you meant chipname when you said target but its available so..

magic_wand_STM32F407VGTx_2020-07-25T18:12:27.848447655-07:00_channel0.txt
magic_wand_STM32F407VGTx_2020-07-25T18:12:27.848447655-07:00_channel1.txt

@jacobrosenthal
Copy link
Contributor Author

Feedback requested

Copy link
Member

@Yatekii Yatekii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now :) Thanks for doing this and sorry it took so long!

@Yatekii
Copy link
Member

Yatekii commented Aug 19, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented Aug 19, 2020

@bors bors bot merged commit 6698282 into probe-rs:master Aug 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants