R4R: Load Ledger Device at Runtime

[alexanderbez]

closes: #2064

---

• Targeted PR against correct branch (see CONTRIBUTING.md)

• Linked to github-issue with discussion and accepted design OR link to spec that describes this work.

• Wrote tests

• Updated relevant documentation (docs/)

• Added entries in PENDING.md with issue #

• rereviewed Files changed in the github PR explorer

---

For Admin Use:

• Added appropriate labels to PR (ex. wip, ready-for-review, docs)
• Reviewers Assigned
• Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

[codecov]

Codecov Report

Merging #2167 into develop will decrease coverage by 0.01%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##           develop   #2167      +/-   ##
==========================================
- Coverage    63.91%   63.9%   -0.02%     
==========================================
  Files          134     134              
  Lines         8194    8194              
==========================================
- Hits          5237    5236       -1     
- Misses        2604    2606       +2     
+ Partials       353     352       -1

[rigelrozanski]

utACK

[ValarDragon]

nit why not do

discoverLedger == nil{
 return error
}
proceed with normal case

just to avoid the extra indent there.

[ValarDragon]

Can you comment that the reason its written as a separate type is to avoid the CGO dependency when the build flag isn't present? (As opposed to having a static function with a boolean for "isPresent")

[cwgoes]

I think we can cache the result of ledger.FindLedger() - unless that caused problems in your testing?

[alexanderbez]

@ValarDragon -- address your comments. Thanks!

@cwgoes -- yes, I intentionally avoided the cache for UX reasons, so when using the LCD, you can exit the app or even disconnect and still have it work upon re-opening/connecting. I can add the cache if you think this should be addressed in another PR or avoided all together.

[cwgoes]

@cwgoes -- yes, I intentionally avoided the cache for UX reasons, so when using the LCD, you can exit the app or even disconnect and still have it work upon re-opening/connecting. I can add the cache if you think this should be addressed in another PR or avoided all together.

I don't think the two are mutually exclusive - just try to refresh the cache if the device isn't valid?

This isn't a big deal though.

[alexanderbez]

Agreed @cwgoes -- for another PR then.

[faboweb]

Great! Will test tomorrow. :)