search

project-chip / rs-matter

Rust implementation of the Matter protocol. Status: Experimental
Apache License 2.0
314 stars 45 forks source link

argument type of `print_pairing_code_and_qr()` #59

Open thekuwayama opened 1 year ago

thekuwayama commented 1 year ago

Hello there,

I have a suggestion regarding the print_pairing_code_and_qr() function. Currently, it accepts comm_data: &CommissioningData as an argument. However, according to section 5.1 of the Matter-1.0-Core-Specification, both MPC and QR Code require a passcode.

The CommissioningData has either VerifierOption::Password or VerifierOption::Verifier. In my understanding, passing VerifierOption::Verifier instead of VerifierOption::Password as an argument to print_pairing_code_and_qr() will result in a runtime error.

To avoid this issue, I recommend passing VerifierOption::Password as the argument to print_pairing_code_and_qr() instead.

nevi-me commented 1 year ago

Hey @thekuwayama , does this also apply in the no_std or sequential branches? @ivmarkov made some changes to the QR code logic in those branches.

ivmarkov commented 1 year ago

@nevi-me @thekuwayama TL;DR: The issue applies to the no_std and sequential branches as well.

Long story: I've not changed how the QR printing code works conceptually. I've only removed allocations from it and made sure that the aspects that do require Rust STD (the actual display of the QR code in the console) are not called / compiled-out for baremetal / no_std.

Related: ideally we should replace the qrcode lib which does need Rust STD with something that is no_std (and ideally - no-alloc - compatible.). Say - this one.

ivmarkov commented 1 year ago

@thekuwayama Would you mind contributing a PR for this?

thekuwayama commented 1 year ago

@ivmarkov OK! :+1: Could you assign me to this?

ivmarkov commented 1 year ago

I can't (no perms). But I guess if you open a PR that would be good enough in terms of an assignment.