{"action":"created","comment":{"url":"https://api.github.com/repos/keyboardio/avr_keyscanner/pulls/comments/192959609","pull_request_review_id":125834094,"id":192959609,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDE5Mjk1OTYwOQ==","diff_hunk":"@@ -192,64 +185,128 @@ void led_set_spi_frequency(uint8_t frequency) {\n     }\n }\n \n+/* Sets all leds off, without using the interrupt */\n+static void _led_all_off_no_interrupt() {\n+    /* make sure we disabled the interrupt ! */\n+    DISABLE_LED_WRITES;\n \n-void led_init() {\n+#define WAIT_SPI_TRANSMIT()     do { while (!(SPSR & _BV(SPIF))) ; } while (0)\n+    SPDR = 0;\n+    for (uint8_t i = 0; i < LED_START_FRAME_BYTES - 1; ++i) {\n+        WAIT_SPI_TRANSMIT();\n+        SPDR = 0;\n+    }\n+    for (uint8_t i = 0; i < NUM_LEDS; ++i) {\n+        WAIT_SPI_TRANSMIT();\n+        SPDR = BRIGHTNESS_MASK | 0;","path":"firmware/led-spiout.c","position":168,"original_position":168,"commit_id":"4e465d4fcd8c39594ffb365d9d110272f8be43a9","original_commit_id":"4e465d4fcd8c39594ffb365d9d110272f8be43a9","user":{"login":"obra","id":45416,"node_id":"MDQ6VXNlcjQ1NDE2","avatar_url":"https://avatars0.githubusercontent.com/u/45416?v=4","gravatar_id":"","url":"https://api.github.com/users/obra","html_url":"https://github.com/obra","followers_url":"https://api.github.com/users/obra/followers","following_url":"https://api.github.com/users/obra/following{/other_user}","gists_url":"https://api.github.com/users/obra/gists{/gist_id}","starred_url":"https://api.github.com/users/obra/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/obra/subscriptions","organizations_url":"https://api.github.com/users/obra/orgs","repos_url":"https://api.github.com/users/obra/repos","events_url":"https://api.github.com/users/obra/events{/privacy}","received_events_url":"https://api.github.com/users/obra/received_events","type":"User","site_admin":false},"body":"I...think we can just say 0 here, no?","created_at":"2018-06-05T06:28:51Z","updated_at":"2018-06-05T06:28:51Z","html_url":"https://github.com/keyboardio/avr_keyscanner/pull/13#discussion_r192959609","pull_request_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/pulls/13","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/keyboardio/avr_keyscanner/pulls/comments/192959609"},"html":{"href":"https://github.com/keyboardio/avr_keyscanner/pull/13#discussion_r192959609"},"pull_request":{"href":"https://api.github.com/repos/keyboardio/avr_keyscanner/pulls/13"}}},"pull_request":{"url":"https://api.github.com/repos/keyboardio/avr_keyscanner/pulls/13","id":192258913,"node_id":"MDExOlB1bGxSZXF1ZXN0MTkyMjU4OTEz","html_url":"https://github.com/keyboardio/avr_keyscanner/pull/13","diff_url":"https://github.com/keyboardio/avr_keyscanner/pull/13.diff","patch_url":"https://github.com/keyboardio/avr_keyscanner/pull/13.patch","issue_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/issues/13","number":13,"state":"open","locked":false,"title":"led spiout improvements","user":{"login":"Gravemind","id":431519,"node_id":"MDQ6VXNlcjQzMTUxOQ==","avatar_url":"https://avatars1.githubusercontent.com/u/431519?v=4","gravatar_id":"","url":"https://api.github.com/users/Gravemind","html_url":"https://github.com/Gravemind","followers_url":"https://api.github.com/users/Gravemind/followers","following_url":"https://api.github.com/users/Gravemind/following{/other_user}","gists_url":"https://api.github.com/users/Gravemind/gists{/gist_id}","starred_url":"https://api.github.com/users/Gravemind/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Gravemind/subscriptions","organizations_url":"https://api.github.com/users/Gravemind/orgs","repos_url":"https://api.github.com/users/Gravemind/repos","events_url":"https://api.github.com/users/Gravemind/events{/privacy}","received_events_url":"https://api.github.com/users/Gravemind/received_events","type":"User","site_admin":false},"body":"I think I improved `led-spiout` a bit trying get more time to run the debouncer.\r\n\r\nHere are the changes I made:\r\n\r\n- Misc optimizations: `SPI_STC_vect`'s switch, remove redundancies, removed some unneeded `volatile` to help the compiler, simplify `leds_dirty` (no data race). I measured(*) a gain of the time of less than half a keyscan at 14 interval, and 1MHz LED SPI.\r\n\r\n- Reduced SPI frequency back to 512kHz, but kept 1MHz just at startup for the first 'all led off'. **This is what helped most**: I measured(*) `chatter-defense` now never miss more than 2 keyscans while rainbow wave is running. (If the 1MHz (1aa3c74ee8) wasn't only to reduce power-up led flashing, then my change is wrong)\r\n\r\n- Reduced _some_ of the led flickering/flashing at power-up: I think some of the flickering was due to a bug in `led_init()`, caught a \"second flashing\" by adding delays in between init steps: it seemed to launched SPI transfer before initializing spi/port/variables (?), so I clarified the code (init global var at declaration, remove redundancies...).\r\n\r\n- Maybe reduced _some_ more flickering with `led_all_off_no_interrupt` hard-coded without using the interrupt so it should be faster (also helps to safely change back the spi freq at startup).\r\n\r\n- Changed `DISABLE_INTERRUPTS` to custom `PROTECT_LED_WRITES` that only disables the LED SPI interrupt, because all the led update functions are called inside interrupts (`TWI_Rx_Data_Callback` is called from the TWI interrupt), and `DISABLE_INTERRUPTS` re-enables the global interrupt flag, which, I think, is bad and could cause interrupt recursion !?\r\n\r\nIt does not seem to change the size of the flasher Sketch or Global variable (weird because in total there is fewer instructions).\r\n\r\n(*): I benchmarked by using a [new debug miss keyscann with leds](https://github.com/Gravemind/avr_keyscanner/commit/led-spiout-work-debug), less overhead but still non-zero, and the code changes a bit when applied on this branch of f/configurable-scanners-and-debouncers ...\r\n\r\n","created_at":"2018-06-03T17:48:01Z","updated_at":"2018-06-05T06:28:51Z","closed_at":null,"merged_at":null,"merge_commit_sha":"f725a406ac5607c1d6d504fde47edf271d63d31f","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/pulls/13/commits","review_comments_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/pulls/13/comments","review_comment_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/pulls/comments{/number}","comments_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/issues/13/comments","statuses_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/statuses/4e465d4fcd8c39594ffb365d9d110272f8be43a9","head":{"label":"Gravemind:led-spiout-work","ref":"led-spiout-work","sha":"4e465d4fcd8c39594ffb365d9d110272f8be43a9","user":{"login":"Gravemind","id":431519,"node_id":"MDQ6VXNlcjQzMTUxOQ==","avatar_url":"https://avatars1.githubusercontent.com/u/431519?v=4","gravatar_id":"","url":"https://api.github.com/users/Gravemind","html_url":"https://github.com/Gravemind","followers_url":"https://api.github.com/users/Gravemind/followers","following_url":"https://api.github.com/users/Gravemind/following{/other_user}","gists_url":"https://api.github.com/users/Gravemind/gists{/gist_id}","starred_url":"https://api.github.com/users/Gravemind/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Gravemind/subscriptions","organizations_url":"https://api.github.com/users/Gravemind/orgs","repos_url":"https://api.github.com/users/Gravemind/repos","events_url":"https://api.github.com/users/Gravemind/events{/privacy}","received_events_url":"https://api.github.com/users/Gravemind/received_events","type":"User","site_admin":false},"repo":{"id":125905211,"node_id":"MDEwOlJlcG9zaXRvcnkxMjU5MDUyMTE=","name":"avr_keyscanner","full_name":"Gravemind/avr_keyscanner","owner":{"login":"Gravemind","id":431519,"node_id":"MDQ6VXNlcjQzMTUxOQ==","avatar_url":"https://avatars1.githubusercontent.com/u/431519?v=4","gravatar_id":"","url":"https://api.github.com/users/Gravemind","html_url":"https://github.com/Gravemind","followers_url":"https://api.github.com/users/Gravemind/followers","following_url":"https://api.github.com/users/Gravemind/following{/other_user}","gists_url":"https://api.github.com/users/Gravemind/gists{/gist_id}","starred_url":"https://api.github.com/users/Gravemind/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Gravemind/subscriptions","organizations_url":"https://api.github.com/users/Gravemind/orgs","repos_url":"https://api.github.com/users/Gravemind/repos","events_url":"https://api.github.com/users/Gravemind/events{/privacy}","received_events_url":"https://api.github.com/users/Gravemind/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/Gravemind/avr_keyscanner","description":"Keyscan controller, with added APA102C LED driver  using an ATtiny88","fork":true,"url":"https://api.github.com/repos/Gravemind/avr_keyscanner","forks_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/forks","keys_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/teams","hooks_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/hooks","issue_events_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/issues/events{/number}","events_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/events","assignees_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/assignees{/user}","branches_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/branches{/branch}","tags_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/tags","blobs_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/git/refs{/sha}","trees_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/statuses/{sha}","languages_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/languages","stargazers_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/stargazers","contributors_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/contributors","subscribers_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/subscribers","subscription_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/subscription","commits_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/commits{/sha}","git_commits_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/git/commits{/sha}","comments_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/comments{/number}","issue_comment_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/issues/comments{/number}","contents_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/contents/{+path}","compare_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/merges","archive_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/downloads","issues_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/issues{/number}","pulls_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/pulls{/number}","milestones_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/milestones{/number}","notifications_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/labels{/name}","releases_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/releases{/id}","deployments_url":"https://api.github.com/repos/Gravemind/avr_keyscanner/deployments","created_at":"2018-03-19T18:48:54Z","updated_at":"2018-03-19T18:48:56Z","pushed_at":"2018-06-04T09:53:36Z","git_url":"git://github.com/Gravemind/avr_keyscanner.git","ssh_url":"git@github.com:Gravemind/avr_keyscanner.git","clone_url":"https://github.com/Gravemind/avr_keyscanner.git","svn_url":"https://github.com/Gravemind/avr_keyscanner","homepage":"","size":8078,"stargazers_count":0,"watchers_count":0,"language":"C","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"mit","name":"MIT License","spdx_id":"MIT","url":"https://api.github.com/licenses/mit","node_id":"MDc6TGljZW5zZTEz"},"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"keyboardio:f/configurable-scanners-and-debouncers","ref":"f/configurable-scanners-and-debouncers","sha":"eefb221d57aabc2fe4d0c39debe47371f1700dc9","user":{"login":"keyboardio","id":13427430,"node_id":"MDEyOk9yZ2FuaXphdGlvbjEzNDI3NDMw","avatar_url":"https://avatars2.githubusercontent.com/u/13427430?v=4","gravatar_id":"","url":"https://api.github.com/users/keyboardio","html_url":"https://github.com/keyboardio","followers_url":"https://api.github.com/users/keyboardio/followers","following_url":"https://api.github.com/users/keyboardio/following{/other_user}","gists_url":"https://api.github.com/users/keyboardio/gists{/gist_id}","starred_url":"https://api.github.com/users/keyboardio/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/keyboardio/subscriptions","organizations_url":"https://api.github.com/users/keyboardio/orgs","repos_url":"https://api.github.com/users/keyboardio/repos","events_url":"https://api.github.com/users/keyboardio/events{/privacy}","received_events_url":"https://api.github.com/users/keyboardio/received_events","type":"Organization","site_admin":false},"repo":{"id":44128289,"node_id":"MDEwOlJlcG9zaXRvcnk0NDEyODI4OQ==","name":"avr_keyscanner","full_name":"keyboardio/avr_keyscanner","owner":{"login":"keyboardio","id":13427430,"node_id":"MDEyOk9yZ2FuaXphdGlvbjEzNDI3NDMw","avatar_url":"https://avatars2.githubusercontent.com/u/13427430?v=4","gravatar_id":"","url":"https://api.github.com/users/keyboardio","html_url":"https://github.com/keyboardio","followers_url":"https://api.github.com/users/keyboardio/followers","following_url":"https://api.github.com/users/keyboardio/following{/other_user}","gists_url":"https://api.github.com/users/keyboardio/gists{/gist_id}","starred_url":"https://api.github.com/users/keyboardio/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/keyboardio/subscriptions","organizations_url":"https://api.github.com/users/keyboardio/orgs","repos_url":"https://api.github.com/users/keyboardio/repos","events_url":"https://api.github.com/users/keyboardio/events{/privacy}","received_events_url":"https://api.github.com/users/keyboardio/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/keyboardio/avr_keyscanner","description":"Keyscan controller, with added APA102C LED driver  using an ATtiny88","fork":true,"url":"https://api.github.com/repos/keyboardio/avr_keyscanner","forks_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/forks","keys_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/keys{/key_id}","collaborators_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/teams","hooks_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/hooks","issue_events_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/issues/events{/number}","events_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/events","assignees_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/assignees{/user}","branches_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/branches{/branch}","tags_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/tags","blobs_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/git/refs{/sha}","trees_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/git/trees{/sha}","statuses_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/statuses/{sha}","languages_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/languages","stargazers_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/stargazers","contributors_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/contributors","subscribers_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/subscribers","subscription_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/subscription","commits_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/commits{/sha}","git_commits_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/git/commits{/sha}","comments_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/comments{/number}","issue_comment_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/issues/comments{/number}","contents_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/contents/{+path}","compare_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/compare/{base}...{head}","merges_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/merges","archive_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/downloads","issues_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/issues{/number}","pulls_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/pulls{/number}","milestones_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/milestones{/number}","notifications_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/labels{/name}","releases_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/releases{/id}","deployments_url":"https://api.github.com/repos/keyboardio/avr_keyscanner/deployments","created_at":"2015-10-12T19:08:32Z","updated_at":"2018-06-05T06:02:47Z","pushed_at":"2018-06-04T09:22:47Z","git_url":"git://github.com/keyboardio/avr_keyscanner.git","ssh_url":"git@github.com:keyboardio/avr_keyscanner.git","clone_url":"https://github.com/keyboardio/avr_keyscanner.git","svn_url":"https://github.com/keyboardio/avr_keyscanner","homepage":"","size":8084,"stargazers_count":9,"watchers_count":9,"language":"C","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":3,"mirror_url":null,"archived":false,"open_issues_count":6,"license":{"key":"mit","name":"MIT License","spdx_id":"MIT","url":"https://api.github.com/licenses/mit","node_id":"MDc6TGljZW5zZTEz"},"forks":3,"open_issues":6,"watchers":9,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/keyboardio/avr_keyscanner/pulls/13"},"html":{"href":"https://github.com/keyboardio/avr_keyscanner/pull/13"},"issue":{"href":"https://api.github.com/repos/keyboardio/avr_keyscanner/issues/13"},"comments":{"href":"https://api.github.com/repos/keyboardio/avr_keyscanner/issues/13/comments"},"review_comments":{"href":"https://api.github.com/repos/keyboardio/avr_keyscanner/pulls/13/comments"},"review_comment":{"href":"https://api.github.com/repos/keyboardio/avr_keyscanner/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/keyboardio/avr_keyscanner/pulls/13/commits"},"statuses":{"href":"https://api.github.com/repos/keyboardio/avr_keyscanner/statuses/4e465d4fcd8c39594ffb365d9d110272f8be43a9"}},"author_association":"NONE"}}
{"actor":{"display_login":"obra"}}