Thursday, 2019-10-24

Thaodanr0kk3rz: why? because I have a newer soc that doesn't build with gcc 4.904: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
r0kk3rzthe fine pine04:42
T42<DylanVanAssche> PinePhone* :p04:47
vknechtpiggz, didn't try resolution pr, can do tonight04:50
Thaodan@DylanVanAssche: I'm not shure if I need something android specific. I never cross compiled kernels before.05:12
ThaodanI found this script to cross compile a gcc tool chain similar to the ones that are uses in android builds05:12
*** electro is now known as Guest8455006:48
T42<DylanVanAssche> Thaodan: This is how we build the kernel using cross compilation:
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
masha11How can i check nfc and fingerprint?07:58
electro575mal, have you an idea for this error ?
T42<birdzhang> switch to root or nemo might helpful08:10
Mister_Magistermal: 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
T42ramsey_22127 was added by: ramsey_2212713:41
vknechtpiggz, 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
vknechtchanging setting has no effect on it15:12
vknechttext only follows setting in image mode, not video15:15
vknechttest build:
T42<adampigg> vknecht good catch, i know what causes that15: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 firewall15:29
T42<DylanVanAssche> mal: PR for Dont Be Evil repo already:
T42<DylanVanAssche> If it's okay, I can make a MR in Gitlab16:54
mal@DylanVanAssche better make it directly to mer-core, of course you can first push it to device specific repo for testing etc16:55
mal@DylanVanAssche a small suggestion, squash those commits to one commit, no need to have a tiny second commit in there16: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 file17:01
maladd those near here
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 them17:02
malI think it's a matter of opinion and style,17:02
malalso if you move those there the naming is not so important17:02
T42<DylanVanAssche> Okay :) Yeah it's indeed style :P17:03
malso the defines look nice and would directly tell in beginning what those are for17:04
T42<DylanVanAssche> mal: You're right :)17:04
mal empty else, you can use the earlier version directly, unless you want to add the parenthesis there17:05
maloops, wrong line17:05
T42<DylanVanAssche> mal: Oh totally missed that one!17:06
malalso use spaces both before and after the operator parts in ternary operators
mal@DylanVanAssche also in other place "else" should be in the same line as the closing curly bracket in "if"s17:08
malas an example and your wrong code
malhmm, seems that the formatting in that code is a mess17:09
T42<DylanVanAssche> mal: that part is a mess, other places are violating that as well17:09
T42<DylanVanAssche> Formatting the whole file is not an option for this MR, that should be another MR17:11
malmaybe do a second commit (before that current one) which fixes those17: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
malat least fix the other new parts now17:12
mal@DylanVanAssche space missing after if
T42<DylanVanAssche> mal: Fixed17:14
malmissed on missing space here  proximityData->value_ = near ? PROXIMITY_NEAR_VALUE: proximityValue;17:16
malbefore that :17:16
T42<DylanVanAssche> Oh also there a space ? :D17:16
mallike I said parts in ternary17:17
malalso rename "near" to like "proximityNear" or something17:17
malnear is too generic17:17
T42<DylanVanAssche> I copied 'near' from hybrisproximity :P17:18
malmaybe it's fine then17:18
T42<DylanVanAssche> I tried to be consistent :)17:18
malin a way those variables could also be inside the case but then it would probably need {} around the content of the case17:19
malI 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 file17:20
mal@DylanVanAssche the magic is {} to make it compile, I have used that sometimes but I think that fine now17: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 case17:23
T42<DylanVanAssche> mal: It is, I don't use it anywhere else17:23
malso I would add the variables into the case with {} around everything except break;17:24
malI'm nitpicking again :)17:25
mal@DylanVanAssche one more, you added extra empty line here
malonly add one empty line17:26
T42<DylanVanAssche> mal: Pushed, fixed the empty line and the {} stuff. I don't mind nitpicking :D17:27
malwondering if the indentation in that case is needed, not very common situation so not sure, I need to think a bit17:28
T42<DylanVanAssche> mal: IMHO it feels wrong if you dont indent. Confuses with if/else and switch17: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 false17:32
malI 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-else17: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
malI usually prefer to have variables in alphabetical order unless these is some grouping needed which make those easier to read17: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
malok, the changes look ok now17:59
T42<DylanVanAssche> mal:
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 stuff18:01
maladd the bug reference to commit message also18:02
malso "[iioadaptor] Implement IIO proximityadaptor. Fixes MER#2062"18:02
mal@DylanVanAssche your branch is behind master, rebase please18:03
malwhen you rebase please change to exactly what I gave for the commit message, that format you use is not the standard18:04
malI mean you changed the . to , and make f small18:05
T42<DylanVanAssche> mal: Fixed, pushing now :)18:05
mallooking better now :)18:05
T42<DylanVanAssche> :D18:05
malas a hint, usually checking some previous commits to see what format is used most of the time is a good thing to do18:07
T42<DylanVanAssche> :) thanks for the tip :D18:07
malit's not always the same format most of the time it's that, in some case it has Contributes to instead of Fixes18:07
T42<DylanVanAssche> Contributes = multiple MR for 1 issue, Fixes = MR fixes the issue18:08
malalso the Fixes is not mandatory though but still often used18:08
malI think it's good to have it18:08
T42<DylanVanAssche> Do we have some page where these contributions tips are listed?18:08
malnot really, I thought it would be common knowledge to follow same commit message as the repo had used in the past18:09
mal*message format18:09
T42<DylanVanAssche> Not only that, but like code style and stuff?18:09
malthe same way everyone should use the same code formatting as the code already in the repo18:09
malcode formatting is not same in all repos, it depends on the history of the repo, who made it first18:10
T42<DylanVanAssche> oh yeah that's difficult in such case18:11
mala bit lower there is a link to coding conventions18:13
malwhich points to this
T42<DylanVanAssche> Oh :O is that added recently? I never saw that page :)18:15
malno sure how old those are18:15
*** electro is now known as Guest9491518:32
T42<adampigg> Vknecht: video resolution fixed19:57
mal@adampigg we are doing orientation things in a bit stupid way, wondering if we should fix those in that or later19:58
malI have a idea for a better way19:59
T42<adampigg> Pr it :)20:03
T42<adampigg> With your cleanups too20: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 directly20: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 tho20:06
T42<adampigg> So,.id like it in soon20:06
malbut 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 not20:07
T42<adampigg> I should really raise an issue for it!20:07
mal@adampigg btw, you use hardcoded magic numbers in your changes20:07
malinstead of the enums available20: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 rotate20:09
T42<adampigg> Yes, ill fix the enums20:10
T42<adampigg> Sry ;(20:10
malI will then later make a PR that switches the _orientation to use the page enums instead of sensor enums for easier comparison20:11
malI 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 there20:12
mal@adampigg also use === for comparisons20:12
malin all places where you use those in new code, I have local code to fix all of those20:13
T42<adampigg> Issue raised20:14
malI probably should finish my cleanup fixes soon20:15
T42<adampigg> I can make the fixes on the bus tomorrow morning. ..away in the evening, ans welding the super5 on sat :D20:15
malotherwise that seems ok, works as expected20: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
T42<adampigg> It should be handled better on the camera state change. That all seema to work better now20:21
T42<adampigg> I was surprised i couldnt query the state in the Loaded status actauklly20:21
malwe should really have a longer discussion and go through the code together to see what could be improved20:21
T42<adampigg> It locked up the said that was when to do it20: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 documented20:22
malprobably gst-droid is doing things incorrectly20:22
T42<adampigg> Atleast now i query in one state, and set ib another20:23
T42<adampigg> It gets rid of a variable20:23

Generated by 2.17.1 by Marius Gedminas - find it at!