{"action":"created","issue":{"url":"https://api.github.com/repos/sheaivey/rx5808-pro-diversity/issues/106","repository_url":"https://api.github.com/repos/sheaivey/rx5808-pro-diversity","labels_url":"https://api.github.com/repos/sheaivey/rx5808-pro-diversity/issues/106/labels{/name}","comments_url":"https://api.github.com/repos/sheaivey/rx5808-pro-diversity/issues/106/comments","events_url":"https://api.github.com/repos/sheaivey/rx5808-pro-diversity/issues/106/events","html_url":"https://github.com/sheaivey/rx5808-pro-diversity/pull/106","id":194815216,"number":106,"title":"WIP: Big tidy up and general refactor","user":{"login":"Knifa","id":622619,"avatar_url":"https://avatars.githubusercontent.com/u/622619?v=3","gravatar_id":"","url":"https://api.github.com/users/Knifa","html_url":"https://github.com/Knifa","followers_url":"https://api.github.com/users/Knifa/followers","following_url":"https://api.github.com/users/Knifa/following{/other_user}","gists_url":"https://api.github.com/users/Knifa/gists{/gist_id}","starred_url":"https://api.github.com/users/Knifa/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Knifa/subscriptions","organizations_url":"https://api.github.com/users/Knifa/orgs","repos_url":"https://api.github.com/users/Knifa/repos","events_url":"https://api.github.com/users/Knifa/events{/privacy}","received_events_url":"https://api.github.com/users/Knifa/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":6,"created_at":"2016-12-11T06:06:58Z","updated_at":"2016-12-15T21:40:46Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/sheaivey/rx5808-pro-diversity/pulls/106","html_url":"https://github.com/sheaivey/rx5808-pro-diversity/pull/106","diff_url":"https://github.com/sheaivey/rx5808-pro-diversity/pull/106.diff","patch_url":"https://github.com/sheaivey/rx5808-pro-diversity/pull/106.patch"},"body":"### Work in progress. Not ready for merge.\r\nMaking PR early for feedback, finding out if it's even worthwhile, etc. If this is to be merged I'll do a rebase and clean up the commits, etc. That said, **every commit is tested and works as expected**. I will keep this description up to date as I go along, too.\r\n\r\nThe code as it stands is pretty cumbersome. Configuration is all over the place, conventions are not followed or consistent, adding new features (not that there's much space) or changing things is difficult, old code left, etc. This is ongoing work to help with that and generally improve code structure, comments, etc. It's likely to end up quite large in the end so I'm happy to split it up. \r\n\r\nChanges so far (in no particular order, yet):\r\n- Improved layout of configuration file so end-users can easily change things to suit their needs. As it stands, there's a bunch of stuff in there that shouldn't really be touched unless the user is making huge modifications.\r\n- General tidying. Variables, includes, defines, etc. that aren't used have been dropped, Names of things have been changed to be more consistent (both in terms of naming and naming convention). Comments have been changed to use correct grammar and be generally more understandable.\r\n- Some work around generalising OLED display code so we don't need so many #ifdefs.\r\n- Improved EEPROM defaults code to check for a magic value at the start. Previous logic was vague.\r\n- Moved PTSR helper out into it's own file, and expanded the function to be more legible.\r\n- Moved channel definitions to their own file.\r\n- Simplified EEPROM usage with structs.\r\n- Simplified receiver SPI command function.\r\n\r\nBig goals:\r\n- Refactor GUI logic to make it way easier to add more screens, and get away from using one huge loop function for handling everything.\r\n- Refactor main loop into smaller functions and files so it's easy to follow.\r\n- Move away from using so many global variables.\r\n- Make everything consistent convention and naming wise.\r\n- ...probably some more!\r\n\r\nHope I'm not stepping on any toes. It's obviously working, and works well, but could just do with a bit of a tidy up! I'd like to have a play and add new things and swap stuff around but I'm finding it pretty difficult to follow just now. :smile:\r\n\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/sheaivey/rx5808-pro-diversity/issues/comments/267452329","html_url":"https://github.com/sheaivey/rx5808-pro-diversity/pull/106#issuecomment-267452329","issue_url":"https://api.github.com/repos/sheaivey/rx5808-pro-diversity/issues/106","id":267452329,"user":{"login":"artemen","id":7785111,"avatar_url":"https://avatars.githubusercontent.com/u/7785111?v=3","gravatar_id":"","url":"https://api.github.com/users/artemen","html_url":"https://github.com/artemen","followers_url":"https://api.github.com/users/artemen/followers","following_url":"https://api.github.com/users/artemen/following{/other_user}","gists_url":"https://api.github.com/users/artemen/gists{/gist_id}","starred_url":"https://api.github.com/users/artemen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/artemen/subscriptions","organizations_url":"https://api.github.com/users/artemen/orgs","repos_url":"https://api.github.com/users/artemen/repos","events_url":"https://api.github.com/users/artemen/events{/privacy}","received_events_url":"https://api.github.com/users/artemen/received_events","type":"User","site_admin":false},"created_at":"2016-12-15T21:40:46Z","updated_at":"2016-12-15T21:40:46Z","body":"there is only one ADC inside the atmel328p, it is internally switched between the various analog pins, the first 10ms after the switch return garbage values, hence I suspect shea uses 50 reads and pulldowns... in reality, this is not necessary, 5 reads is usually enough for this kind of precision as long as there is a 10ms delay between the pin-switch and the first value is discarded. the correct sequence of events is in the code. \r\n\r\nI have my version of diversity controller for some other purposes which includes various types and mixes of RXes, some 0-5v some 0-3.3v and some inverted, to solve this, I use arduino map function. it can be used here as well to get rid of calibration routine for good. \r\n\r\nI am looking forward on you cleaning up the GUI code as I would love to see how its done as I have very limited experience with this type of navigation and would like to implement something like this in my future projects though have very limited time for the hobby. Thanks! "}}
{"actor":{"display_login":"artemen"}}