| r0kk3rz | whyfor? | 02:51 |
|---|---|---|
| Thaodan | r0kk3rz: why? because I have a newer soc that doesn't build with gcc 4.9 | 04:01 |
| T42 | <DylanVanAssche> Thaodan: For the PinePhine we cross compile the mainline kernel. I used GCC 9.3 or 9.2 I think (whatever the latest release is) | 04:27 |
| r0kk3rz | the fine pine | 04:42 |
| T42 | <DylanVanAssche> PinePhone* :p | 04:47 |
| vknecht | piggz, didn't try resolution pr, can do tonight | 04:50 |
| Thaodan | @DylanVanAssche: I'm not shure if I need something android specific. I never cross compiled kernels before. | 05:12 |
| Thaodan | I found this script to cross compile a gcc tool chain similar to the ones that are uses in android builds | 05:12 |
| Thaodan | https://github.com/msfjarvis/build-tools-gcc | 05:12 |
| *** electro is now known as Guest84550 | 06:48 | |
| T42 | <DylanVanAssche> Thaodan: This is how we build the kernel using cross compilation: https://wiki.merproject.org/wiki/Adaptations/PinePhone64#Kernel | 06:52 |
| T42 | <DylanVanAssche> Works fine though :) | 06:52 |
| T42 | <DylanVanAssche> spiiroin: Do you know why the IIO adaptor multiplies the magnetometer on the Z axis by 100? The X and Y axis are not manipulated though. | 07:14 |
| masha11 | How can i check nfc and fingerprint? | 07:58 |
| electro575 | mal, have you an idea for this error ? https://dpaste.de/psjQ | 08:09 |
| T42 | <birdzhang> switch to root or nemo might helpful | 08:10 |
| electro575 | thanks | 08:15 |
| Mister_Magister | mal: ping on my device (fxtec) usb networking is broken aka DHCP doesn't work but after setting ip address manually only telnet works. ping or ssh doesn't work. any idea how can i debug that? | 09:16 |
| T42 | ramsey_22127 was added by: ramsey_22127 | 13:41 |
| vknecht | piggz, hmm, resolution branch: when switching from image to video mode, resolution text still set to 4160x3120 (should be 1920x1080) | 15:08 |
| vknecht | (opening resol settings, seems correctly selected to hd) | 15:09 |
| vknecht | changing setting has no effect on it | 15:12 |
| vknecht | text only follows setting in image mode, not video | 15:15 |
| vknecht | test build: http://repo.merproject.org/obs//nemo:/devel:/hw:/TCL:/idol3/sailfish_latest_armv7hl/armv7hl/harbour-advanced-camera-0.6.1+fix.resolution.switch.20191020214750.3.g8d2e03f-1.38.1.jolla.armv7hl.rpm | 15:16 |
| T42 | <adampigg> vknecht good catch, i know what causes that | 15:25 |
| T42 | <adampigg> mister_magister: its the iptables ... find a command online to clear the iptables rules and it will work .... then, start debugging the firewall | 15:29 |
| T42 | <DylanVanAssche> mal: PR for Dont Be Evil repo already: https://github.com/sailfish-on-dontbeevil/sensorfw/pull/1/files | 16:54 |
| T42 | <DylanVanAssche> If it's okay, I can make a MR in Gitlab | 16:54 |
| mal | @DylanVanAssche better make it directly to mer-core, of course you can first push it to device specific repo for testing etc | 16:55 |
| mal | @DylanVanAssche a small suggestion, squash those commits to one commit, no need to have a tiny second commit in there | 16:56 |
| T42 | <DylanVanAssche> mal: of course, I want merge it first into our device repos to make sure that Adam and other can try it too. | 16:56 |
| T42 | <DylanVanAssche> mal: Okay :) | 16:56 |
| mal | @DylanVanAssche rename the defines to match the other defines, also why are those in header, I doubt those are never used outside .cpp file | 17:01 |
| mal | add those near here https://github.com/sailfish-on-dontbeevil/sensorfw/pull/1/files#diff-b3dcda971f1d807b71380b532931cc8eL54 | 17:01 |
| T42 | <DylanVanAssche> mal: I thought that those defines in the cpp file were at the wrong place :P I'm used to put defines and stuff always in the header :P Will move them | 17:02 |
| mal | I think it's a matter of opinion and style, | 17:02 |
| mal | also if you move those there the naming is not so important | 17:02 |
| T42 | <DylanVanAssche> Okay :) Yeah it's indeed style :P | 17:03 |
| mal | I still would maybe use PROXIMITY_DEFAULT_THRESHOLD and PROXIMITY_NEAR_VALUE | 17:03 |
| mal | so the defines look nice and would directly tell in beginning what those are for | 17:04 |
| T42 | <DylanVanAssche> mal: You're right :) | 17:04 |
| mal | https://github.com/sailfish-on-dontbeevil/sensorfw/pull/1/files#diff-b3dcda971f1d807b71380b532931cc8eL54 empty else, you can use the earlier version directly, unless you want to add the parenthesis there | 17:05 |
| mal | oops, wrong line | 17:05 |
| mal | https://github.com/sailfish-on-dontbeevil/sensorfw/pull/1/files#diff-b3dcda971f1d807b71380b532931cc8eR253 | 17:06 |
| mal | there | 17:06 |
| T42 | <DylanVanAssche> mal: Oh totally missed that one! | 17:06 |
| mal | also use spaces both before and after the operator parts in ternary operators https://github.com/sailfish-on-dontbeevil/sensorfw/pull/1/files#diff-b3dcda971f1d807b71380b532931cc8eR479 | 17:07 |
| mal | @DylanVanAssche also in other place "else" should be in the same line as the closing curly bracket in "if"s | 17:08 |
| mal | as an example https://github.com/sailfish-on-dontbeevil/sensorfw/pull/1/files#diff-b3dcda971f1d807b71380b532931cc8eR420 and your wrong code https://github.com/sailfish-on-dontbeevil/sensorfw/pull/1/files#diff-b3dcda971f1d807b71380b532931cc8eR153 | 17:09 |
| mal | hmm, seems that the formatting in that code is a mess | 17:09 |
| T42 | <DylanVanAssche> mal: that part is a mess, other places are violating that as well | 17:09 |
| T42 | <DylanVanAssche> Formatting the whole file is not an option for this MR, that should be another MR | 17:11 |
| mal | maybe do a second commit (before that current one) which fixes those | 17:11 |
| mal | ok | 17:11 |
| T42 | <DylanVanAssche> Maybe a formatting & clean up MR? There's more issues than just that with the code. | 17:11 |
| T42 | <DylanVanAssche> Debugging statements are also a combination of qDebug() and sensorLogD()/sensorlogW()/... | 17:12 |
| mal | at least fix the other new parts now | 17:12 |
| mal | @DylanVanAssche space missing after if https://github.com/sailfish-on-dontbeevil/sensorfw/pull/1/files#diff-b3dcda971f1d807b71380b532931cc8eR472 | 17:14 |
| T42 | <DylanVanAssche> mal: Fixed | 17:14 |
| mal | missed on missing space here proximityData->value_ = near ? PROXIMITY_NEAR_VALUE: proximityValue; | 17:16 |
| mal | before that : | 17:16 |
| T42 | <DylanVanAssche> Oh also there a space ? :D | 17:16 |
| mal | like I said parts in ternary | 17:17 |
| mal | also rename "near" to like "proximityNear" or something | 17:17 |
| mal | near is too generic | 17:17 |
| T42 | <DylanVanAssche> I copied 'near' from hybrisproximity :P | 17:18 |
| mal | oh | 17:18 |
| mal | maybe it's fine then | 17:18 |
| T42 | <DylanVanAssche> I tried to be consistent :) | 17:18 |
| mal | in a way those variables could also be inside the case but then it would probably need {} around the content of the case | 17:19 |
| mal | I assume hybris proximity sensor did it the same way you did? | 17:20 |
| T42 | <DylanVanAssche> mal: Indeed, I couldn't compile it this way since it's now a jump statement. | 17:20 |
| T42 | <DylanVanAssche> hybrisproximity doesn't have a switch statement since it only has proximity instead of all types in one file | 17:20 |
| mal | @DylanVanAssche the magic is {} to make it compile, I have used that sometimes but I think that fine now | 17:22 |
| T42 | <DylanVanAssche> mal: If it's better to define the variables there, I can move them there, I will use {} then everywhere ? | 17:22 |
| mal | @DylanVanAssche isn't that the only place where you need new variables inside a case | 17:23 |
| T42 | <DylanVanAssche> mal: It is, I don't use it anywhere else | 17:23 |
| mal | so I would add the variables into the case with {} around everything except break; | 17:24 |
| mal | I'm nitpicking again :) | 17:25 |
| mal | @DylanVanAssche one more, you added extra empty line here https://github.com/sailfish-on-dontbeevil/sensorfw/pull/1/files#diff-b3dcda971f1d807b71380b532931cc8eR59 | 17:26 |
| mal | only add one empty line | 17:26 |
| T42 | <DylanVanAssche> mal: Pushed, fixed the empty line and the {} stuff. I don't mind nitpicking :D | 17:27 |
| mal | wondering if the indentation in that case is needed, not very common situation so not sure, I need to think a bit | 17:28 |
| T42 | <DylanVanAssche> mal: IMHO it feels wrong if you dont indent. Confuses with if/else and switch | 17:29 |
| mal | @DylanVanAssche tiny comment, no need to define int proximityValue = 0;, you can define the variable on the same line where you assign the value as the default value is useless anyway, also either remove default from "near" also or optionally remove else from if (proximityValue >= proximityThreshold) as the else use useless when default is already false | 17:32 |
| mal | I think in case of near it's better to remove the else as otherwise you would need to also have variable definition outside of the if-else | 17:34 |
| T42 | <DylanVanAssche> mal: Done! Makes the code a bit more readable :) | 17:38 |
| mal | @DylanVanAssche usually variables are defined first, then other commands after that, so in my opinion the proximityData = proximityBuffer_->nextSlot(); would be after bool near = false; | 17:48 |
| mal | I usually prefer to have variables in alphabetical order unless these is some grouping needed which make those easier to read | 17:49 |
| T42 | <DylanVanAssche> mal: Sure :) I did some cherry picking since it seems that I had a clean up commit in there for the debug logging. Will update the PR :) | 17:51 |
| mal | ok, the changes look ok now | 17:59 |
| T42 | <DylanVanAssche> mal: https://git.merproject.org/mer-core/sensorfw/merge_requests/46 | 18:01 |
| T42 | <DylanVanAssche> Would it be nice to clean up the code together? I can clean up most of it myself, but I might miss some styling stuff. Afterwards we can create another MR to clean up the IIO stuff | 18:01 |
| mal | add the bug reference to commit message also | 18:02 |
| mal | so "[iioadaptor] Implement IIO proximityadaptor. Fixes MER#2062" | 18:02 |
| mal | @DylanVanAssche your branch is behind master, rebase please | 18:03 |
| mal | when you rebase please change to exactly what I gave for the commit message, that format you use is not the standard | 18:04 |
| mal | I mean you changed the . to , and make f small | 18:05 |
| T42 | <DylanVanAssche> mal: Fixed, pushing now :) | 18:05 |
| mal | looking better now :) | 18:05 |
| T42 | <DylanVanAssche> :D | 18:05 |
| mal | as a hint, usually checking some previous commits to see what format is used most of the time is a good thing to do | 18:07 |
| T42 | <DylanVanAssche> :) thanks for the tip :D | 18:07 |
| mal | it's not always the same format most of the time it's that, in some case it has Contributes to instead of Fixes | 18:07 |
| T42 | <DylanVanAssche> Contributes = multiple MR for 1 issue, Fixes = MR fixes the issue | 18:08 |
| mal | yes | 18:08 |
| mal | also the Fixes is not mandatory though but still often used | 18:08 |
| mal | I think it's good to have it | 18:08 |
| T42 | <DylanVanAssche> Do we have some page where these contributions tips are listed? | 18:08 |
| mal | not really, I thought it would be common knowledge to follow same commit message as the repo had used in the past | 18:09 |
| mal | *message format | 18:09 |
| T42 | <DylanVanAssche> Not only that, but like code style and stuff? | 18:09 |
| mal | the same way everyone should use the same code formatting as the code already in the repo | 18:09 |
| mal | code formatting is not same in all repos, it depends on the history of the repo, who made it first | 18:10 |
| T42 | <DylanVanAssche> oh yeah that's difficult in such case | 18:11 |
| mal | @DylanVanAssche https://sailfishos.org/wiki/Collaborative_Development#Commit_policies | 18:13 |
| mal | a bit lower there is a link to coding conventions | 18:13 |
| mal | which points to this https://sailfishos.org/wiki/Coding_Conventions | 18:14 |
| T42 | <DylanVanAssche> Oh :O is that added recently? I never saw that page :) | 18:15 |
| mal | no sure how old those are | 18:15 |
| *** electro is now known as Guest94915 | 18:32 | |
| T42 | <adampigg> Vknecht: video resolution fixed | 19:57 |
| mal | @adampigg we are doing orientation things in a bit stupid way, wondering if we should fix those in that or later | 19:58 |
| mal | I have a idea for a better way | 19:59 |
| T42 | <adampigg> Pr it :) | 20:03 |
| T42 | <adampigg> With your cleanups too | 20:03 |
| mal | @adampigg if we use page.orientation more we can use page.orientation & Orientation.PortraitMask for checking if device is portrait or inverted portrait, why do we even have the _orientation when? | 20:04 |
| mal | *when we probably could just use page.orientation directly | 20:04 |
| T42 | <adampigg> Fair point.... Also, i recorded a video ib the pro1, and when i played it back, it was upside down :) | 20:06 |
| T42 | <adampigg> The resolution pr fixes a real bug tho | 20:06 |
| T42 | <adampigg> So,.id like it in soon | 20:06 |
| mal | but that would maybe cause some issues if we use it directly because it would be affected by orientation lock, do we want our UI to follow orientation lock or not | 20:07 |
| T42 | <adampigg> I should really raise an issue for it! | 20:07 |
| mal | @adampigg btw, you use hardcoded magic numbers in your changes | 20:07 |
| mal | instead of the enums available | 20:08 |
| T42 | <adampigg> No, i dont think we should honour lock? | 20:08 |
| mal | @adampigg our UI is quite different anyway as the layout doesn't really change with rotation, just the icons rotate | 20:09 |
| T42 | <adampigg> Yes, ill fix the enums | 20:10 |
| T42 | <adampigg> Sry ;( | 20:10 |
| mal | I will then later make a PR that switches the _orientation to use the page enums instead of sensor enums for easier comparison | 20:11 |
| mal | I think code would easier to read with _orientation & Orientation.PortraitMask instead of two comparison, although that does make the sensor reading code a bit more complex but just a simple switch-case there | 20:12 |
| mal | @adampigg also use === for comparisons | 20:12 |
| mal | in all places where you use those in new code, I have local code to fix all of those | 20:13 |
| T42 | <adampigg> Issue raised | 20:14 |
| mal | I probably should finish my cleanup fixes soon | 20:15 |
| T42 | <adampigg> I can make the fixes on the bus tomorrow morning. ..away in the evening, ans welding the super5 on sat :D | 20:15 |
| mal | ok | 20:15 |
| mal | otherwise that seems ok, works as expected | 20:16 |
| mal | @adampigg you removed applySettings(); from ModeSwitch onClicked, is that now done in some other place? maybe in the camera status handling? | 20:19 |
| T42 | <adampigg> Might consider making more of the ui reflect actual state instead of setting.. | 20:19 |
| mal | true | 20:20 |
| T42 | <adampigg> It should be handled better on the camera state change. That all seema to work better now | 20:21 |
| T42 | <adampigg> I was surprised i couldnt query the state in the Loaded status actauklly | 20:21 |
| mal | we should really have a longer discussion and go through the code together to see what could be improved | 20:21 |
| T42 | <adampigg> It locked up the camera...docs said that was when to do it | 20:21 |
| mal | @adampigg I have noticed that true, when I was trying to handle the camera switching long time ago it problematic because nothing worked as Qt had documented | 20:22 |
| mal | probably gst-droid is doing things incorrectly | 20:22 |
| T42 | <adampigg> Atleast now i query in one state, and set ib another | 20:23 |
| T42 | <adampigg> It gets rid of a variable | 20:23 |
Generated by irclog2html.py 2.17.1 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!