Add simulator for local development and testing#878
Add simulator for local development and testing#878KaustubhPatange wants to merge 10 commits intoSeedSigner:devfrom
Conversation
* Simulator that works on macos * fork simulator from https://github.com/ltcmweb/seedsigner * Fix camera exposure and screen size * resize and crop + revert rbga in read_capture * remove padding * move gpio to different module * restore modules for dev mode * merge camera * extract local display --------- Co-authored-by: Hector Chu <hectorchu@gmail.com> Co-authored-by: Nick Klockenga <nick@klockenga.net>
Can you provide more detail about this? I've never used any of the emulators so I'm not familiar enough to understand why your changes on top of the other emulator are necessary. Is the original emulator code not sufficient on its own? What is it lacking? What are its shortcomings? |
src/seedsigner/models/psbt_parser.py
Outdated
|
|
||
| for out in self.psbt.outputs: | ||
| _fill_scope(out) No newline at end of file | ||
| _fill_scope(out) |
There was a problem hiding this comment.
Unrelated formatting changes should not be included in a feature PR. It's trivial, obviously, but it pollutes the PR and makes it a little less likely to get reviewed if someone sees that a large number of files have been changed when part of that count is just whitespaces or other unrelated minor / unnecessary edits.
|
|
||
| This guide adds a method to setup SeedSigner testing locally through a simulator GUI for easy development. | ||
|
|
||
| It was tested on macOS (26) using Homebrew and Python 3.14. With additional effort it would work on other operating systems and python version or it may still way directly I've not tested it as I don't own any other devices. |
There was a problem hiding this comment.
Documentation should not take on an "I" voice. Documentation is speaking on behalf of the entire project.
You can simply list the limitations. Something like: "Note: this has only been tested on macOS."
And then provide further explanation in the PR description ("I didn't have any other devices to test this on"). Then during PR review, hopefully others will test on their machines and you can expand the documentation according to their feedback.
| ```bash | ||
| git clone https://github.com/SeedSigner/seedsigner | ||
|
|
||
| python3.14 -m venv env |
There was a problem hiding this comment.
See our project's supported Python versions.
There was a problem hiding this comment.
Actually, I guess there's a hole in our pyproject.toml.
But see:
seedsigner/.github/workflows/tests.yml
Line 23 in d557aff
requirements.txt
Outdated
| embit | ||
| Pillow | ||
| pyzbar @ git+https://github.com/seedsigner/pyzbar.git@c3c237821c6a20b17953efe59b90df0b514a1c03 | ||
| qrcode==7.3.1 | ||
| urtypes==1.0.1 | ||
| qrcode | ||
| urtypes |
There was a problem hiding this comment.
It is unacceptable to unpin our specific version dependencies.
@kdmukai Before I reply / implement fixes to other questions / code points could you point to other emulators? Are you talking about running the code directly within Raspberry pi devices? |
You wrote in the description: The original simulator code was taken from https://github.com/ltcmweb/seedsigner. |
Got it, this is not an emulator but a simulator. An emulator runs the operating system and then the code. A simulator simulates the operating system feature like display or camera. The way this simulator works is it creates a tkinter UI directly like a native application on your operating system as a display, uses laptop camera or webcam as camera module for the application. No need for emulator, just run and a window will be created. This native desktop window simulates the gpio pin inputs through buttons just like a actual display attached to raspberry Pi. The original code where this simulator was taken / inspired replaced existing display drivers and removed a bunch a code that break raspberry builds. So I restored them and added a launcher gui.py script if you want to run the app natively on a desktop or use main.py if you are running on a raspberry device. The benefit of doing this is we can easily prototype software changes quickly without needing an actual device in hand. I hope this answer your question :) |
I had to dig up the link to the emulator I was thinking of: https://github.com/enteropositivo/seedsigner-emulator Can you be more specific about how your work differs from enteropositivo's work? I maybe conceptually barely understand the "emulator" vs "simulator" distinction, but, again, my lack of familiarity with enteropositivo's original work is a barrier here for me. In other words: I'm not well versed on what has been done before nor do I understand how or why this PR differs from that prior work. I DO think it's way overdue for us to officially incorporate an emulator/simulator option, but right now I don't know if we should be focusing on enteropositivo's work or your PR or something else. |
|
@kdmukai Thanks for sharing the repository. Didn't knew people were already making simulators. I looked at the repo and turns out it has the exact same business logic that this PR has, the difference is the one you linked replaces the files within source code and my version has both mode.
Makes sense, looks like we already have sufficient options from other people's work just not visible enough unless someone searches for it. The decision to have this officially within seedsigner is your choice. You can close this PR as not planned in that case. enteropositivo's work and this PR is similar, just that one replaces code and mine tries to combine them both so builds produced from os should still work on raspberry pi. Thanks for quick responses. |
|
I think I dislike either approach:
We've made creative use of |
This sounds like a good idea. Using mock we can patch modules at run time when a launcher script like I can mark the PR draft and re-implement the approach as you suggested. Might take a while though as I'll be balancing this alongside my full time job. |
|
@kdmukai Turns out this was more easy than expected since the hard part was already done. The new code now introduces Please take a look whenever you are available. I can also rebase the commits if needed. |
|
I still haven't done a close review, but sounds like exciting progress @KaustubhPatange!
Are the similarities straightforward enough that you could PR your One thing that might help: I think the override classes in run_emulator.py would be better pulled out into their own simple .py files. Similarly, if your PR into the enteropositivo repo can mostly re-use the python files that are already there (avoiding massive refactors or copying them into a single file), then it would be a much easier PR for enteropositivo to review and merge. I ask because enteropositivo's emulator is the one that has been most commonly used so far by other devs. So if any emulator becomes "official", I'd like it to be that one based on its track record and as a kind of sign of respect, but with the improved approach here. This also aligns with my opinion that any official emulator should exist as a separate repo from the main SeedSigner code. So we would either fork the enteropositivo repo or ask if they would be willing to transfer repo ownership. Depending on the timing, it could maybe be:
|
|
@kdmukai The similarities differ with entropositivo's repo. The setup for emulator with entropositivo's says that we first must clone the seedsigner repo and then download emulator repo and replace files within seedsigner. The problem is entro's repo replaces renderer file which leads to few problems maintaining compatibility. Example PR 15 from entro's repo: enteropositivo/seedsigner-emulator#15 which says any significant changes to renderer in seedsigner will break the emulator. It also does not support macOS from what it seems enteropositivo/seedsigner-emulator#12 (although this looks like a setup issue). Entro's work uses virtual gpio inputs (dict based) which requires lot of replacing files in seedsigner code. The approach this PR presents is to actually emulate a display rather than renderer using a local file socket so it basically behaves like a new display we supported. So even if renderer code is updated it will not affect emulator. This is the reason Entro's work cannot be entirely patched. Good news is the other parts like camera, buttons is the same code that
I'm aligned with extracting classes into multiple files for easy maintainability but regarding to emulator being a different repo doesn't sound good. Emulator requires seed signer code to be present without that it cannot run. Entro's repo doesn't contain any seedsigner code instead his work is applied on top of seedsigner. So in a way if we combine those we should be maintaining a single repo with seedsigner and emulator. Another reason why this is a good idea is we can write test to verify that any PR merges doesn't break emulator & if it does changes should be made to fix emulator. With different repo we wouldn't know if emulator is breaking unless someone tests it or have to run periodic github action to check which sounds overkill. Also for other developers who look in contributing to seedsigner doesn't have to go over a different repo just to setup an emulator for quick prototyping (similar to anyone testing seed phrase to qr using If we consider emulator as a utility or a tool we should keep it inside seedsigner If the issue is with credibility that Entro's name should be there with the emulator, then one of the approach that we can do is all the similar files that this PR contains should credit back to Entro's work with comments at the top. Update the simulator setup README to give Entro's work credit and explain how this solution is built on top of his work. We can also ask him to review the PR although I don't think Entro is active anymore so maybe optional? But I think with proper comments we can credit this PR back to Entro's original work. If this sounds good I can start implementing these changes. |
|
Okay, summing up:
|
We use git submodule approaches elsewhere already. Our entire l10n system won't run without the So a standalone emulator repo could do the same. Or, perhaps it'll be even easier to just recommend setting up a symlink to your local working dir for the main repo. So as someone changes their main repo's local branch / fork / PR they're testing, there's nothing to coordinate on the emulator side. And I'd reverse the framing:
We are moving toward pulling more and more out of the main repo (e.g. the 3d printable enclosures). The goal is to really focus the main repo to minimize clutter, attack surface area, etc. See the work going on in the It'll also be a bit easier to get PRs merged for emulator-only changes if it's in its own repo. Main repo changes are high-stakes and so reviewers are less eager to jump into any PR for the main repo. They might not be any harder to review, but there's a higher mental barrier to even getting started, if that makes sense.
We already have some CI Github Actions on all PRs in the main repo. One of them includes generating screenshots in at least one language that depends on the I'd also imagine that over time we'll have more and more people testing PRs first with the emulator, so that would surface some obvious issues. But since the emulator is not part of the Production stack, I'd be okay if it does end up being occasionally broken due to a recent main repo merge. Not a great answer, obviously. But given the project's history, I just don't think we'll have many unexpected breaking changes in the core areas that affect input, screen rendering, etc. |
|
@kdmukai Reading this through, it makes sense to decouple as much as possible from seedsigner repo into a self contained repo to what is functioning as a mono repo currently. So the idea I'm thinking is that we will have a separate repo let's say In SeedSigner official repo we will have docs that point to this emulator repo. Not entirely sure whether it will have a submodule of emulator as well to warn in CI whether the changes broke the emulator. The issue is having two repo be submodule of each others will work but is considered as a bad practice. What do you suggest? BTW those changes you suggested with appropriate credits are pushed. |
I don't think so. If you look into some of the CI workflows, it's basically like a Dockerfile / scripted VM. We can just script in whatever steps we need. Or note how the main repo is referenced in the So we have full flexibility to use repo X in repo Y regardless of how X relates to Y via submodules or vice versa or at all. |
|
@kdmukai So it behaves like a submodule without a submodule, just cloning repos internally. Sounds good, so how will this work. Will you be creating a repo or want to me to do the whole implementation in a separate repo and transfer to SeedSigner? So step by step approach would be,
|
We have a devs' call coming up Thursday. I'll add this to the agenda. You are welcome to join. If you aren't already in our Telegram group, start there: https://t.me/seedsigner_new_devs |
I would split this into separate tasks:
I think it'll be easier and faster to review just the emulator and its test suite. Not everyone (hardly anyone!) is an expert in the CI workflow scripting. So that's better tackled in a separate PR so that it doesn't hold up the main PR.
Reordered for priority.
This CI update would probably just run the emulator's test suite, basically re-using (or copying from) the CI above. There isn't a good way to implement the warning since CI can only return success or failure. I know there are workflows that can update comments in the PR. That might make the most sense here. But that might get too ambitious; I'm not familiar at all with how those are implemented. |
|
@SeedSigner can you create a new @KaustubhPatange once that new repo is up, you can open a PR there for the emulator. In our dev call today we suggested:
As discussed above, I strongly recommend that the initial PR be as compact and limited as possible. You can even open other PRs for each incremental expansion, if any make sense. |
|
I am in telegram channel just couldn't join the call but I'll wait till the new repo is setup so I can continue raising incremental PRs. Thanks @kdmukai |
|
@kdmukai Looks like the repository is not created yet. Let me know when it is available. |
|
@KaustubhPatange I have realised that it's not necessary to run the gui in a separate process from the main script. Simply run the main function in a thread and launch the gui within the main script. This allows to get rid of the socket IPC. Also prefer to use Qt. I have implemented these changes in my simulator: ltcmweb@d3a8546 |
|
@KaustubhPatange the new repo has been created. https://github.com/seedsigner/seedsigner-emulator Sorry for the delay. |
|
Raised a PR SeedSigner/seedsigner-emulator#1 in the new repo. We can close this PR and continue over there. |
4 participants
Description
This PR adds a simulator to seedsigner that can help with local testing running with tkinter for GUI and opencv for camera operations. The simulator is tested on a macbook (macos 26) but it should work with other platforms as well.
This simulator is useful for quick local testing any change without needing an actual raspberry pi device to be connected all the time. In theory this simulator could also help to drive end to end feature testing with screenshots.
The original simulator code was taken from https://github.com/ltcmweb/seedsigner.
I made few modifications on top of it so that it can work in two modes RPI and Local desktop. For screenshot review thedocs/imgfolder.If there should be any change needed do let me know.
Edit: Based on feedback #878 (comment), the simulator now replaces modules using mock implementation rather than overwriting code files.
This pull request is categorized as a:
Checklist
pytestand made sure all unit tests pass before submitting the PRIf you modified or added functionality/workflow, did you add new unit tests?
I have tested this PR on the following platforms/os: