Skip to content
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

plugin-meye-config & extension-meye #73

Open
wants to merge 34 commits into
base: main
Choose a base branch
from

Conversation

AdamVasarhelyi2
Copy link

Presenting a new plugin (plugin-meye-config) and extension (extension-meye) for review. Steps to verify functionality:

In cmd.exe:

  1. Run npm install in the root directory

  2. Run npm run build inside packages/plugin-meye-config and packages/extension-meye

  3. Navigate to /packages (i.e., go up a directory)

  4. Run a local python server with python -m http.server

  5. In your web browser, navigate to http://localhost:8000/

  6. To test the plugin, navigate to /plugin-meye-config/examples. To test the extension, navigate to /extension-meye/examples. Note that the extension example calls the index.browser.min.js file from the plugin's /dist directory as the plugin and extension are to be used together.

  7. Once the examples finish, they output data that you can examine.

The original website for the technology implemented by the plugin / extension is https://www.pupillometry.it/

In both the original mEye and the plugin, it is difficult for calibration to be perfect (it is harder in the original website because calibration must be done manually, but has been automatized in the plugin). If testing is difficult due to lighting constraints, you can print out a picture of an eye and hold it up to the camera. If you put your hand over the pupil, the extension's data for blink probability should increase to 1.

Please let me know if you have any questions :)

@changeset-bot
Copy link

changeset-bot bot commented Jul 27, 2023

⚠️ No Changeset found

Latest commit: 1182d07

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@jodeleeuw
Copy link
Member

Hi @AdamVasarhelyi2,

Thanks for the contribution!

It looks like there are files in dist/ folders that are being committed here. These should be ignored via .gitignore. I'm wondering if they aren't being caught because you uploaded them directly. Can you remove these from the PR?

@AdamVasarhelyi2
Copy link
Author

Sure Josh! Thanks for the feedback - I'm a little new :P

I'll get this done by Monday

@AdamVasarhelyi2
Copy link
Author

Hi @AdamVasarhelyi2,

Thanks for the contribution!

It looks like there are files in dist/ folders that are being committed here. These should be ignored via .gitignore. I'm wondering if they aren't being caught because you uploaded them directly. Can you remove these from the PR?

Hey Josh sorry for taking forever, I'm in a super hectic part of my last semester at college. I'll get around to this soon :)

@AdamVasarhelyi2
Copy link
Author

Hey @jodeleeuw, I've finally gotten around to this. Sorry for taking so long!! I hope everything's okay now - I (think I) have removed the files from the /dist directory that were causing problems with the first submission. Please let me know if everything is okay!

@jodeleeuw jodeleeuw changed the title Add files via upload plugin-meye-config & extension-meye Jul 9, 2024
@jodeleeuw
Copy link
Member

@AdamVasarhelyi2, sorry for letting this slip through the cracks! I've updated a few things to match existing conventions. The next step is getting the package-lock.json to update, but I don't have permission to push directly to your fork for that. Can you pull my changes and then run npm i in the root of the repository to generate an updated package-lock.json file? When you push that I think the build should work.

@AdamVasarhelyi2
Copy link
Author

AdamVasarhelyi2 commented Jul 9, 2024 via email

@AdamVasarhelyi2
Copy link
Author

AdamVasarhelyi2 commented Jul 14, 2024 via email

@jodeleeuw jodeleeuw changed the base branch from main to add-cli July 14, 2024 13:28
@jodeleeuw jodeleeuw changed the base branch from add-cli to main July 14, 2024 13:28
@jodeleeuw jodeleeuw requested a review from jadeddelta October 29, 2024 18:30
Copy link
Collaborator

@jadeddelta jadeddelta left a comment

Choose a reason for hiding this comment

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

hey, this plugin and extension looks like a fantastic idea and we'd love to include it. i've added some code review notes for you to view at your leisure, but also just one important distinction is that i haven't actually been able to run the code, no matter what I do, apparently my browser just isn't powerful enough, but i will continue to try and run it later on and let you know, these are just some helpful tips for you. thanks again for the contributions!

packages/extension-meye/examples/index.html Show resolved Hide resolved
name: "meye-extension",
};

// I'm sorry for this ;_; but hey it works
Copy link
Collaborator

Choose a reason for hiding this comment

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

it's okay @ - @;;, but it would be nice to type these values just for intellisense. i think vscode + typescript can infer types if possible, it's just to help with debugging and maintainability

packages/extension-meye/src/index.ts Show resolved Hide resolved
packages/extension-meye/src/index.ts Show resolved Hide resolved

const info = <const>{
name: "meye-config",
parameters: {},
Copy link
Collaborator

Choose a reason for hiding this comment

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

it would be good to have parameters that specify stuff like the paths to images in case that people have their own images they want to upload (different languages, etc)...

Copy link
Author

@AdamVasarhelyi2 AdamVasarhelyi2 Dec 15, 2024

Choose a reason for hiding this comment

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

I don't quite understand. The plugin configures the extension which measures pupils in real time. Why would one want to configure real-world video with an uploaded image over real-world video? And what did you mean with the language thingy?

introduction();
};

function introduction() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

i'm wondering if it's just worth leaving this out and instead having a user just use an html-button-response or instructions plugin beforehand: it would also give them more power to customize this prompt

Copy link
Author

Choose a reason for hiding this comment

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

I really wanted it to be part of the plugin because they're special instructions that are unique to the plugin and are unlikely to change or be different between research. The reason it's unlikely to change is because orthogonal lighting is better for it and outdoor lighting is orthogonal. I can still change this if you prefer, though.

packages/plugin-meye-config/src/index.ts Show resolved Hide resolved
packages/plugin-meye-config/src/index.ts Show resolved Hide resolved
packages/plugin-meye-config/css/main.css Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants