weixin_39563132的博客

Yeah Exactly. Even I was not satisfied with the following solution but it made it work so I ignored it. Apart from using odometry sampling ratio, is there no other way to fix this since I do need the odometry data along with LiDAR for my application?

Hi, Sorry the for the delayed response I did the Validation this time as mentioned. Here is the screenshot for the bag file. So I followed like what you suggested that there must be multiple publishers on the clock topic, so I removed --clock from my launch file. <node name="playbag" pkg="rosbag" type="play" args="$(find cartographer_ros)/bag/new_test.bag">

And indeed it was the issue and so no more warning messages. So everything looks good.

The only thing is in order to run the Cartographer I'm using a low value for odometry_sampling_ratio as mentioned earlier. Thanx alot everyone.

Yes, I do use --clock. You can see my node playbag (3rd line from the bottom) in the launch file i posted earlier.

So, I recorded my bag once again including the /clock as well this time. I ran the same configuration by setting use_odometry = true and odometry_sampling_ratio = 0.1 and it is working now. But I'm getting warning messages as you can see in the image. I know the error which I was getting was due to the difference in the rate at which data was being published in Gazebo and when launched with Cartographer, it throws an error. Maybe this is not a good workaround but it worked for me. Please correct me if I'm wrong.

Awesome, Thanks! Actually the subdivision and accumulation was exactly the reason why I wanted the scans for each sensor to be separate. But I didn't know that I need to adjust TRAJECTORY_BUILDER_2D.scans_per_accumulation to the number of range finders

I sent you a mail with the data.

Ok i'll prepare that

, yes we also have this issue as mentioned. But its not directly related to this one. It's just that odometry solves this issue when its not broken due to other problems (like PR 499 in our case).

Alternatively I can repost my suggestions there. I just opened a new issue, because I wanted to discuss a potential implementation with the goal of preparing a pull request afterwards.

Please take another look

I just noticed that i messed sth. up when resolving merge conflicts :/ I will take care of it on monday

Hey, sorry for the delay I was on vacation the last 3 weeks. I hope I didn't overlook anything

Thanks for the review! It was quite hard to remember why things where confusing for me when I started to read through the code... :)

Another question about 2.: Can you elaborate on the heavy filtering? The discussion in https://github.com/googlecartographer/cartographer_ros/pull/411 indicates that you want to be able to use old scans for scan matching. Somehow I fail to see how you what to do it without the scan_data. Or to be more precise: I can't think of a way of filtering, that is preserving the scan matching, but inhibits the drawing of submaps.

1. The submaps are slightly skewed so if you would improve the alignment on the lower end (in the screenshot) it would come apart on the other end.
2. Ok, I see your point there. But for some use cases the accuracy of the map might be more constraining then the memory usage. Would you consider to implement the dropping as an optional feature?
3. That's actually very interesting then. So lets say we load this old Trajectory with one single big submap and use it for localization. In the new trajectory we would not only keep the last 3 submaps, but also all the submaps that show significant change compared to the patch in the big old map below. After some time we would then bake these "update submaps" into the big old one. For this – and to avoid degenerating the big map over time – we would again need to replay the scans along the optimized trajectory. Or am I missing something here?
4. Don't be sad :) Its likely a feature that many in the ROS community would love to see. It's just that we hope to use cartographer for mapping as well as for localization in the future. At the moment we in fact use the yaml and pgm from version of cartographer before https://github.com/googlecartographer/cartographer_ros/commit/99da2f20d8ce9a7c9549f68315635b644168a14b to patch small updates into our map. Localization is still done with mrpt

Ok maybe i'm too focused about this potential solution already. I will take a step back and explain the problem.

We want to do live-long-slam. Our approach would be to keep a set of submaps that covers the whole area and to remove those submaps that are to some degree covered by newer submaps. (hence #409 ) The problem we see is, that our submaps contain an error from drifting that creates discontinuous and/or double walls when stitching them together. I think the "cartographer way" to improve this would be to reduce the submap size. But we have already quite small ones and performance seams to reduce if we go much lower. By intuition I would also guess that the repetitiveness of the environments where our robots drive dictates a lower bound on the submap size.

Anyway, loop closures were working well after some tuning, so the OccupancyGrid code, that was recently removed, created a very decent map – much better than stitching together the imperfect submaps. So we thought to improve the submaps probability_grid using the same process of replaying the respective scans along the optimized trajectory. On second thought though; question to you : Would this also improve the scan matching against the submap? To my understanding scans are matched against the probability grid, so improving it should improve the matches. After all what we want to achieve is improved localization. Nice looking maps are just considered an indicator for reliability.

About your questions: 1. The output of asset_writer main does in fact look much better. The double walls are gone (see attached images, esp. lower end) 2. Ok I was mostly referring to the following snippet. Could you explain what exactly is going to be removed? I'm not sure if i got you right

 C++
  range_data_inserter.Insert(carto::sensor::TransformRangeData(
        Decompress(node.constant_data->range_data),
        node.pose.cast<float>()),
                               &probability_grid);
  }
</float>

3. Thanks for the pointer I was not even aware of this code and the option to define actions for the asset_writer. However: if we would want to improve the scan matching a "single large (sub)map" would not help... would it? Or are you suggesting to replace a couple of submaps with such a optimized bigger submap? And then to use this for localization? That could actually work for us as well. 4. Sounds cool, but this was not initially our aim when we opened this issue

____ Attached images

Did anyone start to work on this? Otherwise I would start by trying to reuse the old BuildOccupancyGrid2D code for it.

Done

Ok ready for another round :)

Please take another look :)

Hey would you like to explain the use case for those manual constraints?

Well thank you for testing and looking into this, it's really appreciated!

Also if you can't test by making the JS changes (it's more tedious to do so I know) you can use this, I don't think (not sure) it works well when composer is minimized, but it at least allows you to test when the composer is open on mobile :) without having to touch the JS

css-less
 (max-width: 767px) {
    .Composer:not(.minimized) {
        position: fixed;
        top: 0 !important;
    }
}

This works great! So, I guess the idea behind having it stuck to the top instead of stuck to the bottom has to do with previewing and tiny screens? My brain is like... hmm, having it at the top is fine. But, comment boxes are usually at the bottom of UIs, no? Regardless I’m going with this CSS now at my sites until there’s something permanent. I’m sure there are very good reasons for going with sticking to top versus bottom. And, I’m sure the JS change works too by the way. If it works for you, I don’t see why it wouldn’t work on iOS.

I like the pop up... but, sometimes I wonder if there’s a better fully responsive way. Because it’s not fully responsive right? That’s why we’re having the screen size issues? And having to rely on JavaScript to position it well?

That's what this pull request does, removes the need to position it with javascript, and fixes it to the screen instead. Currently the behavior is an absolute and a set position with JS, so that one can see the discussion behind while scrolling (but there is no need for that since you can minimize the composer anyway). The composer element itself is responsive because it adapts to a better view on smaller screens, it's just that because of the absolute position (changed to fixed in this PR) it creates the 2 linked to issues.

Ok, thanks for explaining! I didn’t get it at first. I’ll hold my breath and cross my figures that all of the devs agree with this pull request for beta 16. I would think it needs to be in beta 16 to ensure full testing before stable.

Sorry, no I didn’t do a pull request. Just commenting that the original pull request probably won’t work.

It's hard to tell if you haven't also applied the js changes if this is a bug with the PR or not:

Original solution above, using fixed, for me completely hides composer now.

This may be best. This ensures the composer is at the bottom, similar to how it is on desktop.

That looks nice on your mobile screen, but how does it behave on smaller mobile screens where there is no space for both the keyboard and the composer ? I imagine you wouldn't be able to see the title field anymore.

Also, fixed positioning fixes a problem where the composer is stuck at a specific position in the screen, creating issues when closing the keyboard and other scenarios: https://discuss.flarum.org/d/24831-ui-bug-mobile-postdiscussion-is-not-on-the-screen-sometime

Another way of looking at this is: Unless a better fix can be found relatively quickly.. Should Flarum be a good experience on super tiny screens or a good experience on very popular iPhone screens?

I don't understand what you mean, we are not forced to choose between both, this pull request fixes all issues.

I didn’t look at the pull request. I didn’t realize it included JavaScript too. Sounds great! At least I have a CSS patch for my sites for now which seems to work. I’ll probably do some tiny screen tests later to see what all gets hidden.

I like the pop up... but, sometimes I wonder if there’s a better fully responsive way. Because it’s not fully responsive right? That’s why we’re having the screen size issues? And having to rely on JavaScript to position it well?

I’m sure there are really good reasons. But, I wonder why the post box on mobile isn’t just like the post box on desktop? Where it is responsive and slides up into view.

I think there might be the risk that the composer gets hidden by the browser header on small screens, right?

Maybe. But, that’s a risk even if it is left as-is isn’t it? You could also make the composer height a little smaller. It’s close to 250px as-is. It could be changed to 200px or 220px.

Another way of looking at this is: Unless a better fix can be found relatively quickly... Should Flarum be a good experience on super tiny screens or a good experience on very popular iPhone screens? I know both... maybe eventually. But, the very popular iPhone is overlooked too much, IMO.

did you only apply the LESS changes or did you also apply JS changes (i.e: all changes done by this PR) ?

Sorry, no I didn’t do a pull request. Just commenting that the original pull request probably won’t work.

Original solution above, using fixed, for me completely hides composer now.

This may be best. This ensures the composer is at the bottom, similar to how it is on desktop. It makes the most sense. Composer box adjacent to your virtual keyboard. "But, we want people to be able to scroll and see behind it," some say. 1) You can still scroll up and see other posts. 2) After typing enough content you start to see your new post behind the post box without scrolling. 3) There is the eyeball 👁 preview button. People should use that to see their post before posting. There is very limited real estate on mobile with the keyboard expanded. We should no longer play the “must see everything behind it” game when it comes to mobile.

~~~

(max-width: 767px) { .Composer:not(.minimized) { top: initial !important; bottom: 0 !important;

}

} ~~~

Looks like this:

http://snippi.com/s/txcushr

Slim v2.0.1.

Here's an example http://d.pr/f/1LHG

It boils down to the fact that Flarum uses a lot of hover effects and hover causes double tapping on touch devices. I know it may seem boring, but I personally try to avoid hover effects all together these days. But I also like very minimal design.

There’s a link to possible solutions above where Franz mentions (pointer: fine). Or, there’s these: https://stackoverflow.com/questions/23885255/how-to-remove-ignore-hover-css-style-on-touch-devices

A nuclear option is ignoring all hover styles completely on smaller screens. As long as that wouldn’t break anything.

Or, the CSS I posted above, although it may not be glamorous, works great (at least for the discussion double tap and as long as no one ever removes the “no-touch” class from core).

iPhone XR. Landscape is: 1792 x 828 px

The thing is, there could always be a phone out there now or in the future that has just the right amount of pixels to reproduce this. Ensuring the drop down toggle is hidden for mobile fixes this no matter the width/pixels.

I also think this is more important than the notifications issue since this could prevent someone from opening a discussion. The most basic part of a forum.

Could you confirm?

No this is not fixed. It has always worked in portrait mode. But on iPhone as one example when you rotate the phone to landscape, it takes a double tap to open a discussion.

I still have to use the following CSS patch to fix it:

~~~ / Fix double tap to open discussion on touch devices / .DiscussionListItem .Dropdown-toggle.Button { display: none; } .no-touch .DiscussionListItem .Dropdown-toggle.Button { display: block; } ~~~

I remember reading about this somewhere. It's intended behavior in iOS, though it can get annoying depending on the use case. Android doesn't do this, as it fires the click event on the first tap as well as hover. Further reading: https://css-tricks.com/annoying-mobile-double-tap-link-issue/

I also sometimes wonder if the last point in this article is the way to go... "It's probably best to just not rely on hover to reveal anything." Hover makes the desktop experience a little better and "cooler," but if one wants to 100% ensure their website works well on all devices with no extra taps, it seems like hover's gotta go. That'd be a pretty big change for Flarum though so probably not a good idea to go that route. :)

I found out that this is only an issue if you have not uploaded an avatar yet. It has something to do with the code involved with there not being an avatar. Because if you manage to get an avatar uploaded, say via a desktop computer, then this is no longer an issue on iOS.

Also, if you double tap on the original letter avatar, then it works.

I wonder if this could be the solution. Use the label tag: https://stackoverflow.com/questions/21842274/cross-browser-custom-styling-for-file-upload-button

This is still an issue. I’m on iOS 11.4.1, using Chrome version 68.0.3440.83, which I assume is the latest version as of today.

If I tap the plus sign just right, the iOS upload box slides up as it should. But, it only works every now and then.

I think this is a fairly common problem with iOS and upload buttons if you aren’t using a standard upload button.

Again it’s very inconsistent. So you may think this is no longer an issue one day, then a week later try to upload and it doesn’t work.

Solved. Is this the permanent fix? Maybe not. Maybe transform needs to be completely removed or adjusted some other way. But, here’s a fix. It is now app-navigation that is the culprit.

.App-navigation {transform: initial;}

Here’s video evidence. This video will show me recreating the two jumping issues on my phone. Then it will show me editing the CSS, and then it shows me unable to reproduce the issue since transform:initial fixes it.

https://vimeo.com/492702913

Confirming this is still an issue even going into beta 15. But, on top of the top nav bar disappearing, there is also an issue with the tag selector drawer menu on mobile. For my beta 14 installations, for reasons I personally do not fully understand, the following fixes both issues.

.App:before { transform: initial; }

For beta 15 and higher, maybe (not tested yet) the following could work:

.App-header { transform: initial !important; }

See this thread for video of the issue:

https://discuss.flarum.org/d/25669-two-big-issues-solved-with-one-css-rule

I tried to reproduce this in the latest versions of Chrome and Firefox for iOS and Android as well as Safari on iOS and Samsung's browser for Android. Unfortunately, I was unable to reproduce this issue even with the steps to reproduce provided in this issue.

Yes, sometimes it’s very hard to reproduce. Other times I get it to happen several times in a row. Here is a gif so that you can fully see what I mean. This shows the demo site, and then the discuss.flarum site as well. This is Chrome iOS, latest version.

• I read about the transform rule here: https://stackoverflow.com/a/10170170 ...notice the first comment under this answer. It seems this rule can fix these types of issues, but also can cause them.

Here are some other people talking about it. Even though these say to add this rule, not remove it, all of these types of posts led me to the idea of removing the rule. Again, since I removed the rule, I can no longer reproduce this at my forum.

https://stackoverflow.com/q/50105780

https://stackoverflow.com/a/18764086

https://stackoverflow.com/a/25923988

I will admit though that this is a very small bug and it may be a while until someone out there can reproduce it to help verify.

I wonder if this transform rule has always been there or if it was added to fix a different bug? So far, I don’t see anything negative happening as a result of me removing this rule at my forum.

I can't think of any sites that allow username logins anymore

Twitter? Or am I thinking about that wrong? Maybe that’s not technically a username and/or, I’m sure being Twitter they find other ways to harden security? I get trying to keep usernames more private for security. But, I do think some sites, even some big ones, still have username logins and display usernames. You just need strong passwords. Maybe 2 factor eventually.

Anyway... just saw that comment and thought I’d foolishly for whatever reason go to bat for sites that display usernames and allow username logins. 🙃

I’m actually a fan of going with email only logins or whatever is decided. Or, let the user decide how secure they want to be. A switch in the admin to allow or not allow username logins. I guess that could be an extension. I also guess I’m getting a little off topic.

Well shoot. Just when I thought I somewhat figured this out... I did see that the post content is still in the database, so I guess the content not being on the frontend for me is an extension conflict.