-
Notifications
You must be signed in to change notification settings - Fork 221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: ledger stax + flex support #6588
Conversation
Test Results (CI) 3 files 129 suites 35m 46s ⏱️ For more details on these failures, see this check. Results for commit 22db1ff. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some changes, and questions.
I don't love that this is how they want us to write apps. I'd almost be more inclined to write two different ones, and modularize all the functional bits so we just wrap it in two different UX's.
but the work is done now, and it is how they suggest doing it so 🤷🏻
#[cfg(any(target_os = "stax", target_os = "flex"))] | ||
{ | ||
// Load glyph from 64x64 4bpp gif file with include_gif macro. Creates an NBGL compatible glyph. | ||
const FERRIS: NbglGlyph = NbglGlyph::from_include(include_gif!("key_64x64.gif", NBGL)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with FERRIS
What's an NBGL campatible glyph anyway? What's NBGL?
@@ -199,6 +202,7 @@ pub fn handler_get_script_offset( | |||
comm.append(&[RESPONSE_VERSION]); // version | |||
comm.append(&script_offset.to_vec()); | |||
offset_ctx.reset(); | |||
offset_ctx.finished = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this to do with your state issue?
What exactly is the bool solving, I think I'm overlooking its purpose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an attempt at getting the UI working, good catch this needs to be removed
/// Allocator heap size | ||
const HEAP_SIZE: usize = 1024 * 26; | ||
|
||
/// Statically allocated heap memory | ||
static mut HEAP_MEM: [MaybeUninit<u8>; HEAP_SIZE] = [MaybeUninit::uninit(); HEAP_SIZE]; | ||
|
||
/// Bind global allocator | ||
#[global_allocator] | ||
static HEAP: embedded_alloc::Heap = embedded_alloc::Heap::empty(); | ||
|
||
/// Error handler for allocation | ||
#[alloc_error_handler] | ||
fn alloc_error(_: core::alloc::Layout) -> ! { | ||
SingleMessage::new("allocation error!").show_and_wait(); | ||
ledger_device_sdk::exit_app(250) | ||
} | ||
|
||
/// Initialise allocator | ||
pub fn init() { | ||
unsafe { HEAP.init(HEAP_MEM.as_ptr() as usize, HEAP_SIZE) } | ||
} | ||
|
||
struct MyCriticalSection; | ||
critical_section::set_impl!(MyCriticalSection); | ||
|
||
unsafe impl critical_section::Impl for MyCriticalSection { | ||
unsafe fn acquire() -> RawRestoreState { | ||
// nothing, it's all good, don't worry bout it | ||
} | ||
|
||
unsafe fn release(_token: RawRestoreState) { | ||
// nothing, it's all good, don't worry bout it | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this tested on the nanos+ ?
I was under the distinct impression without defining the allocator we lose the ability to well... Allocate anything. Essentially it's memory-less
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes it was,
Ledger realised that they need one, so the rust_sdk lib now includes one and it clashes as you can only have one defined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh neat.
Co-authored-by: Brian Pearce <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack
Description
Upgrading ledger library for stax and flex support