{"action":"created","issue":{"url":"https://api.github.com/repos/esp8266/Arduino/issues/5493","repository_url":"https://api.github.com/repos/esp8266/Arduino","labels_url":"https://api.github.com/repos/esp8266/Arduino/issues/5493/labels{/name}","comments_url":"https://api.github.com/repos/esp8266/Arduino/issues/5493/comments","events_url":"https://api.github.com/repos/esp8266/Arduino/issues/5493/events","html_url":"https://github.com/esp8266/Arduino/issues/5493","id":390921714,"node_id":"MDU6SXNzdWUzOTA5MjE3MTQ=","number":5493,"title":"Rewrite PUYA patch to be more universal and mem friendly.","user":{"login":"TD-er","id":3751318,"node_id":"MDQ6VXNlcjM3NTEzMTg=","avatar_url":"https://avatars3.githubusercontent.com/u/3751318?v=4","gravatar_id":"","url":"https://api.github.com/users/TD-er","html_url":"https://github.com/TD-er","followers_url":"https://api.github.com/users/TD-er/followers","following_url":"https://api.github.com/users/TD-er/following{/other_user}","gists_url":"https://api.github.com/users/TD-er/gists{/gist_id}","starred_url":"https://api.github.com/users/TD-er/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/TD-er/subscriptions","organizations_url":"https://api.github.com/users/TD-er/orgs","repos_url":"https://api.github.com/users/TD-er/repos","events_url":"https://api.github.com/users/TD-er/events{/privacy}","received_events_url":"https://api.github.com/users/TD-er/received_events","type":"User","site_admin":false},"labels":[{"id":194072129,"node_id":"MDU6TGFiZWwxOTQwNzIxMjk=","url":"https://api.github.com/repos/esp8266/Arduino/labels/component:%20core","name":"component: core","color":"fad8c7","default":false},{"id":210716775,"node_id":"MDU6TGFiZWwyMTA3MTY3NzU=","url":"https://api.github.com/repos/esp8266/Arduino/labels/staged-for-release","name":"staged-for-release","color":"5319e7","default":false},{"id":192608393,"node_id":"MDU6TGFiZWwxOTI2MDgzOTM=","url":"https://api.github.com/repos/esp8266/Arduino/labels/type:%20enhancement","name":"type: enhancement","color":"84b6eb","default":false}],"state":"closed","locked":false,"assignee":{"login":"devyte","id":16824838,"node_id":"MDQ6VXNlcjE2ODI0ODM4","avatar_url":"https://avatars0.githubusercontent.com/u/16824838?v=4","gravatar_id":"","url":"https://api.github.com/users/devyte","html_url":"https://github.com/devyte","followers_url":"https://api.github.com/users/devyte/followers","following_url":"https://api.github.com/users/devyte/following{/other_user}","gists_url":"https://api.github.com/users/devyte/gists{/gist_id}","starred_url":"https://api.github.com/users/devyte/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/devyte/subscriptions","organizations_url":"https://api.github.com/users/devyte/orgs","repos_url":"https://api.github.com/users/devyte/repos","events_url":"https://api.github.com/users/devyte/events{/privacy}","received_events_url":"https://api.github.com/users/devyte/received_events","type":"User","site_admin":false},"assignees":[{"login":"devyte","id":16824838,"node_id":"MDQ6VXNlcjE2ODI0ODM4","avatar_url":"https://avatars0.githubusercontent.com/u/16824838?v=4","gravatar_id":"","url":"https://api.github.com/users/devyte","html_url":"https://github.com/devyte","followers_url":"https://api.github.com/users/devyte/followers","following_url":"https://api.github.com/users/devyte/following{/other_user}","gists_url":"https://api.github.com/users/devyte/gists{/gist_id}","starred_url":"https://api.github.com/users/devyte/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/devyte/subscriptions","organizations_url":"https://api.github.com/users/devyte/orgs","repos_url":"https://api.github.com/users/devyte/repos","events_url":"https://api.github.com/users/devyte/events{/privacy}","received_events_url":"https://api.github.com/users/devyte/received_events","type":"User","site_admin":false}],"milestone":{"url":"https://api.github.com/repos/esp8266/Arduino/milestones/8","html_url":"https://github.com/esp8266/Arduino/milestone/8","labels_url":"https://api.github.com/repos/esp8266/Arduino/milestones/8/labels","id":1894520,"node_id":"MDk6TWlsZXN0b25lMTg5NDUyMA==","number":8,"title":"2.5.0","description":"","creator":{"login":"igrr","id":4349050,"node_id":"MDQ6VXNlcjQzNDkwNTA=","avatar_url":"https://avatars2.githubusercontent.com/u/4349050?v=4","gravatar_id":"","url":"https://api.github.com/users/igrr","html_url":"https://github.com/igrr","followers_url":"https://api.github.com/users/igrr/followers","following_url":"https://api.github.com/users/igrr/following{/other_user}","gists_url":"https://api.github.com/users/igrr/gists{/gist_id}","starred_url":"https://api.github.com/users/igrr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/igrr/subscriptions","organizations_url":"https://api.github.com/users/igrr/orgs","repos_url":"https://api.github.com/users/igrr/repos","events_url":"https://api.github.com/users/igrr/events{/privacy}","received_events_url":"https://api.github.com/users/igrr/received_events","type":"User","site_admin":false},"open_issues":8,"closed_issues":230,"state":"open","created_at":"2016-07-20T16:16:06Z","updated_at":"2019-01-04T01:45:24Z","due_on":"2019-02-03T08:00:00Z","closed_at":null},"comments":27,"created_at":"2018-12-14T00:51:11Z","updated_at":"2019-01-04T09:08:19Z","closed_at":"2018-12-19T04:00:36Z","author_association":"CONTRIBUTOR","body":"The (current) patch:\r\n1. uses a static 4KB buffer, which means everyone using the patch has 4KB less heap, including those users who do _not_ have a puya chip. Instead, a dynamically allocated buffer should be used.\r\n2. evaluates the first if condition every single call of the function, although only the first one after boot actually does anything. Instead of a private static, a global could be used that is set elsewhere only once at bootup, and then if condition is no longer needed. Alternatively, The ESPClass could get a new data member (i.e.: instead of the global flag) which is set on construction, and a method to return its value.\r\n3. calculates the flash_chip_id condition in the second if every single call of the function, which makes no sense, because it can't ever change at runtime. Instead, the condition should be fully calculated once and the result stored in a flag, which should then be checked.\r\n4. detects the condition based on flash_chip_id, which isn't very robust, as has been shown with the flash chip size. Instead, a general purpose detection approach should be implemented that transparently covers other potential brands and manufacturers that have the same quirk, should they appear.\r\nExample: on first bootup after flashing, choose a sector, e.g.: within the update area of the flash layout., preferably one that is unlikely to be used. Do a write test and read back sequence known to behave differently with puya chips. If puya is detected, set a quirk flag somewhere. On all successive bootups, detect that the quirk flag is set, so don't do the test again. Use the quirk flag in the write function to decide whether the additional logic is done or not.\r\n\r\nGiven the above, the patch won't be accepted as-is into our core. There have been several internal discussions on exactly what to do, but given that the patch is available, there have been, and still are, higher priorities, e.g.: figuring out how to ease IRAM usage.\r\n\r\nThere have been a lot of comments from many users in this discussion, and I see the patch is even being applied elsewhere, but I still don't see a PR here. Instead of wondering what will happen, I invite those involved to make a PR with the proposed patch, and implement at least points 1-3 above on top of it. If you come up with a good solution for point 4, even better.\r\n\r\n_Originally posted by @devyte in https://github.com/esp8266/Arduino/issues/4061#issuecomment-447076931_\r\n\r\n\r\nWill have a look at it tomorrow (well, after sleep)"},"comment":{"url":"https://api.github.com/repos/esp8266/Arduino/issues/comments/451390147","html_url":"https://github.com/esp8266/Arduino/issues/5493#issuecomment-451390147","issue_url":"https://api.github.com/repos/esp8266/Arduino/issues/5493","id":451390147,"node_id":"MDEyOklzc3VlQ29tbWVudDQ1MTM5MDE0Nw==","user":{"login":"TD-er","id":3751318,"node_id":"MDQ6VXNlcjM3NTEzMTg=","avatar_url":"https://avatars3.githubusercontent.com/u/3751318?v=4","gravatar_id":"","url":"https://api.github.com/users/TD-er","html_url":"https://github.com/TD-er","followers_url":"https://api.github.com/users/TD-er/followers","following_url":"https://api.github.com/users/TD-er/following{/other_user}","gists_url":"https://api.github.com/users/TD-er/gists{/gist_id}","starred_url":"https://api.github.com/users/TD-er/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/TD-er/subscriptions","organizations_url":"https://api.github.com/users/TD-er/orgs","repos_url":"https://api.github.com/users/TD-er/repos","events_url":"https://api.github.com/users/TD-er/events{/privacy}","received_events_url":"https://api.github.com/users/TD-er/received_events","type":"User","site_admin":false},"created_at":"2019-01-04T09:08:19Z","updated_at":"2019-01-04T09:08:19Z","author_association":"CONTRIBUTOR","body":"@daniftodi It is included in the staging version of core 2.5.0\r\nThe important lines are:\r\n- Define PUYA_SUPPORT=1  (`-DPUYA_SUPPORT=1`)\r\n- Use `platform                  = https://github.com/platformio/platform-espressif8266.git#feature/stage`  (like @uzi18 pointed out)\r\n\r\nI will try to make a git branch on my own repository, so it is included in the older core versions too.\r\nThis makes it easier to build patched versions on core 2.4.x (and 2.3.x)"}}
{"actor":{"display_login":"TD-er"}}