search

microsoft / pxt-microbit

A Blocks / JavaScript code editor for the micro:bit built on Microsoft MakeCode
https://makecode.microbit.org
Other
721 stars 590 forks source link

Radio does not work between board variants without first setting the group #3421

Closed abchatra closed 1 year ago

abchatra commented 4 years ago
image

Does not work v2-v1 Works v1-v1 Works v2-v2

Setting the group in on start fixes this, but it should work without having to do this

pelikhan commented 4 years ago

The issue here is that the hash of the program is different between nrf51 and nrf52; which leads to a different random radio group. SOlutions are:

mmoskal commented 4 years ago

We could embed the project guid in the generated code, and use that as the group. Then the group wouldn't change as the user modifies program, but is different from other users.

abchatra commented 4 years ago

My vote is "always use group 254" and document it. If someone doesn't use a group, it is always 254. Easy to remember. No kid understands the hash logic.

Most folks set their group anyway.

@Jaqster @jaustin

Jaqster commented 4 years ago

Why not always default to group 1? easier to explain...

abchatra commented 4 years ago

Why not always default to group 1? easier to explain...

There might be bunch of scripts out there with group 1 which might conflict if default group is also 1. Unlikely there are many scripts with group 254. Only for backward compat reasons. Otherwise group 1 is better.

Jaqster commented 4 years ago

How would the scripts conflict? Wouldn't it just work anyways - whether group 1 is set explicitly by the user or default-under-the-covers? To me, having the default group setting be something obvious and easy to explain is better... image

pelikhan commented 4 years ago

The point of this feature was to avoid unexpected packet collisions between all the random projects running on micro:bit in a classroom. At this point, if we are to pick a number, we could just default to 1 and always start the tutorial with setting up the group number.

abchatra commented 4 years ago

This is the scenario I am thinking of: "One kid sets the group to 1 and flashes 2 microbits" "Another kid sets to nothing and flashes 2 microbits"

They wouldn't conflict today due to the hash algorithm. They would if you keep the default group as 1.

We could say that is ok. As setting to group 1 is a good solution if we don't have to worry about the backward compat.

Jaqster commented 4 years ago

That's okay IMO. Most times the group setting is optional - kids/teachers expect messages to flow to all micro:bits in the classroom. I've seen Teachers say "if you want to send a secret message" then they have the students set the group number to something secret that only the sender/receiver know.

abchatra commented 4 years ago

Sounds good to me. Lets set to default group to "1"

jaustin commented 4 years ago

I'm really worried about defaulting to any group.

It means collisions happen far more often in the classroom. We directly see the downside of this In MicroPython where there isn't the hashing, people spend a lot of time at the beginning of workshops wondering what/why things are triggereing inadvertantly

This is also a significant behaviour change because a lesson that has group-work with no setting of the group will now not work.

While it's feasible for you to change all MakeCode tutorials to include setting the group all the time, it's not possible to change the books and lesson code around the place that does NOT do this and it's a new and confusing bug for people.

jaustin commented 4 years ago

That's okay IMO. Most times the group setting is optional - kids/teachers expect messages to flow to all micro:bits in the classroom. I've seen Teachers say "if you want to send a secret message" then they have the students set the group number to something secret that only the sender/receiver know.

I don't think the expectation that messages go to all devices is something I've seen as common - some very popular activities like "Chuck-a-duck" are predicated on this not being the case.

@microbit-giles do you have thoughts on this from your experience with teachers and in the classroom yourself?

@mmoskal said:

We could embed the project guid in the generated code, and use that as the group. Then the group wouldn't change as the user modifies program, but is different from other users.

This seems like an easy way to keep the current behaviour across the two devices. What's the downside?

abchatra commented 4 years ago

Expectation is going to some device though. Hash will ensure the code will go to no device. Unless you flash from the same PC.

abchatra commented 4 years ago

We have group setting in all our tutorials

Jaqster commented 4 years ago

The way I've seen this explained is that the Teacher talks about how a Radio works - a sender broadcasts messages out and receivers pick it up and hear the message. Then they do a "send my name" activity in the classroom. It's mahem and the students love it. Then sometimes, the Teacher will do the "secret message" activity where they set the Radio group.

Jaqster commented 4 years ago

I think the hash is confusing and difficult to explain behavior. Agree this would be a change to the API, but I think it's an improvement and much clearer to explain to Teachers. We could wait on this until the next major release...

pelikhan commented 4 years ago

Many lessons tell user to change group to 1 and 1 is the default so better pick another one.

From: Abhijith Chatra notifications@github.com Sent: Wednesday, October 14, 2020 6:11:46 PM To: microsoft/pxt-microbit pxt-microbit@noreply.github.com Cc: Peli de Halleux jhalleux@microsoft.com; Comment comment@noreply.github.com Subject: Re: [microsoft/pxt-microbit] Radio does not work between board variants without first setting the group (#3421)

Why not always default to group 1? easier to explain...

There might be bunch of scripts out there with group 1 which might conflict if default group is also 1. Unlikely there are many scripts with group 254. Only for backward compat reasons. Otherwise group 1 is better.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Fpxt-microbit%2Fissues%2F3421%23issuecomment-708505906&data=04%7C01%7Cjhalleux%40microsoft.com%7C5e7011f792b74b0fdba308d8705bd999%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637382887078573052%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=5IiCLb7qHnamiqyzzDcS%2FO3W8wUyERjzHsXf9FRneNs%3D&reserved=0, or unsubscribehttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAA73QKIIWQEKF6DNSAT6GC3SKXEUFANCNFSM4SPLPNFQ&data=04%7C01%7Cjhalleux%40microsoft.com%7C5e7011f792b74b0fdba308d8705bd999%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637382887078583008%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=bJxtuc1ucu%2BmsVPrG6mhD4scogF0CM2Jq4J5LQ8C52U%3D&reserved=0.

Jaqster commented 4 years ago

Many lessons tell user to change group to 1 and 1 is the default so better pick another one.

Why does this matter?

pelikhan commented 4 years ago

Getting random packets from any micro:bit is equally confusing. This issue can be solved by systematically introducing the concept of group in radio lessons. Also, we could display the group on the simulator.

pelikhan commented 4 years ago

Why does this matter?

To avoid collisions with random program that haven't set the group number.

Jaqster commented 4 years ago

Why does this matter?

To avoid collisions with random program that haven't set the group number.

But if we clearly explain that "if you don't set a Radio Group, then it defaults to channel 1" is a lot easier for people to understand than "there is a secret hash code for programs with no radio group set - you can't see it, and we can't tell you which programs are compatible"

jaustin commented 4 years ago

Expectation is going to some device though. Hash will ensure the code will go to no device. Unless you flash from the same PC.

But this is the behaviour today and it really doesn't confuse people as far as I see?

If you want two programs to communicate from different computers, set the group...

abchatra commented 4 years ago

It does confuse people. I have talked to so many who don't understand the default behavior. I just say set the group.

abchatra commented 4 years ago

But if we clearly explain that "if you don't set a Radio Group, then it defaults to channel 1" is a lot easier for people to understand than "there is a secret hash code for programs with no radio group set - you can't see it, and we can't tell you which programs are compatible"

Teaching a new behavior is not easy. People need to read documentation again. That is the reason I thought "Group 254" is a good alternative. It will work for anyone who doesn't set the group and doesn't conflict with the usually set group of number 1-10.

jaustin commented 4 years ago

Why does this matter?

To avoid collisions with random program that haven't set the group number.

But if we clearly explain that "if you don't set a Radio Group, then it defaults to channel 1" is a lot easier for people to understand than "there is a secret hash code for programs with no radio group set - you can't see it, and we can't tell you which programs are compatible"

I think the explanation I like for this is "if you don't set the group, you get a special group just for your program so it doesn't interfere with other programs in the room"

Without the hash - a teacher now has to allocate a different group to each student or group of students to stop them colliding! Before if you had students working on the same PC or groups working on one PC, that problem completely went away.

That's a additional overhead for a teacher if they're trying to help debug, because you can't be sure someone else hasn't got the same group - Firstly asking if anyone else is using the same group, then checking they haven't actually got it wrong in their code, etc.

abchatra commented 4 years ago

I disagree @jaustin. Everything should work by default for kids. Primary purpose of radio is talk to another kids microbit. If it works just for one kid, is not a good default.

jaustin commented 4 years ago

It does confuse people. I have talked to so many who don't understand the default behavior. I just say set the group.

I think that's fine though. I think confused but NON colliding is a lot better than confused AND colliding. The thing about a room where the group is set to the same default is that you see really odd behaviour you can't undestand.

abchatra commented 4 years ago

Hash can be any number including 0-9. So you will get message sometimes. Explaining that is hard.

jaustin commented 4 years ago

I think we're discussing a change in behaviour because of a bug we can fix... why don't we fix the bug and then have a separate discussion about the change in behaviour?

Fixing the bug: seems like there's a viable way to do it listed in https://github.com/microsoft/pxt-microbit/issues/3421#issuecomment-708441434 that and keeps consistent behaviour. Let's do that first, because it is the least change for people.

Changing the behaviour: Let's do some research, have a wider discussion and make a breaking change at an annual boundary, or sooner if there's overwhelming evidence.

Jaqster commented 4 years ago

I think we need more user testing and to talk with Educators. What I've seen has obviously been very different from what you're seeing Jonny. I have never seen students with more than 1 micro:bit each, and most of the scenarios I've seen have been "Broadcast to everyone" or "Send a secret message".

jaustin commented 4 years ago

Hash can be any number including 0-9. So you will get message sometimes. Explaining that is hard.

Yea, that's definitely true. Hashes colliding, or colliding with definitively selected groups is one of the downsides. In practice I've never seen this. But it's confusing as you say, because it breaks the truth of "you have a code just for your program"!

jaustin commented 4 years ago

I think we need more user testing and to talk with Educators. What I've seen has obviously been very different from what you're seeing Jonny. I have never seen students with more than 1 micro:bit each, and most of the scenarios I've seen have been "Broadcast to everyone" or "Send a secret message".

Yes, agree with some wider user testing and discission with educators - also it's probably now quite a daunting thread for new people to grok and chime in with as we're escalating the post count pretty quickly! :)

pelikhan commented 4 years ago

Settings the hash from a "computer PC id" could also be reasonable. It will work with your working buddy but it's unlikely that you get bothered by other people's scripts.

abchatra commented 4 years ago

Lets do user study and do the right thing. We tend to keep arguing in this point and do no progress. I oppose any new hash.

jaustin commented 4 years ago

Thinking a bit more broadly here are some wider suggestions that aren't just 'have a hash or have a fixed default'.

I don't think any of these is complete or quite nails it, but they're different to what I've thought about before on this...

Jaqster commented 4 years ago

I like these ideas. Agree that making the group setting more explicit would help clear up a lot of confusion when it comes to explaining the "Radio Magic"...

pelikhan commented 4 years ago

Showing the invisible state - eg show group id - on the simulator is best - regardless of the default.

From: Jonathan Austin notifications@github.com Sent: Wednesday, October 14, 2020 7:10:33 PM To: microsoft/pxt-microbit pxt-microbit@noreply.github.com Cc: Peli de Halleux jhalleux@microsoft.com; Comment comment@noreply.github.com Subject: Re: [microsoft/pxt-microbit] Radio does not work between board variants without first setting the group (#3421)

Thinking a bit more broadly here are some wider suggestions that aren't just 'have a hash or have a fixed default'.

I don't think any of these is complete or quite nails it, but they're different to what I've thought about before on this...

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Fpxt-microbit%2Fissues%2F3421%23issuecomment-708538560&data=04%7C01%7Cjhalleux%40microsoft.com%7Ce58c20058ba5434996cb08d87064100c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637382922349316712%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=b%2Bi3PU3zPh4fYb24g4gsP5kOcaAGs%2F5Yhlw%2Fr%2F6XSI4%3D&reserved=0, or unsubscribehttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAA73QKPUZD5CNALGF3TAJATSKXLQTANCNFSM4SPLPNFQ&data=04%7C01%7Cjhalleux%40microsoft.com%7Ce58c20058ba5434996cb08d87064100c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637382922349326704%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=%2BgACxEn6xi%2FNZ8X14Ho9XdGsUo%2FRN7I5GXGRQjZ0gas%3D&reserved=0.

microbit-giles commented 4 years ago

I think we're discussing a change in behaviour because of a bug we can fix... why don't we fix the bug and then have a separate discussion about the change in behaviour?

I agree with @jaustin that we have two different things here. The micro:bit is all about continuity and stability, and this is especially sensitive as we introduce V2. We should preserve the existing behaviour at least for now, and fix it so the hash works across V1 and V2 devices.

Changes in the radio behaviour may be a good idea but would best be incorporated into future updates and not linked in users' minds with the introduction of V2 hardware.

microbit-katie commented 4 years ago

I agree with @pelikhan to show the invisible state - by showing the group ID.

Can we do this with a 'radio set group' block that automatically appears in an 'on start' loop of the scripting area?

It will default set the radio group, but allow the learner to change it if they wish. @Jaqster @abchatra

microbit-mark commented 4 years ago

Setting the radio group was also discussed back in https://github.com/microsoft/pxt-microbit/issues/2017 and closed as we decided not to break compatibility at the time and discuss for the next release. Can we set the hash so it works between board variants for now as a workaround and then look to introduce a default radio group for the 2021 MakeCode release?

Here's an example in the wild of someone testing radio between board variants, who either debugged and found the workaround or was already setting the group in their program https://twitter.com/sw_education/status/1318111398690316288

microbit-mark commented 3 years ago

@abchatra @Jaqster is this one being discussed for the upcoming release?

abchatra commented 1 year ago

We made a change to set the default group to zero and there have been no complaints over 9 months now. Closing.