Focus chart screen to aid IPD/focus adjustment

Request to add an option to display a full screen "Back Focusing Chart" image (black & white) to help with manual IPD/Focus adjustment.

Select "Focus Chart" in goggle menu (to be added)
Display full screen image of "Back Focusing Chart" - its a black and white chart wheel in the center and small ones in each corner
Allow user to adjust mechanical IPD/Focus nobs on goggle
Use a button to return to the goggle menu, return to menu

The image should be stored in firmware (not on SD card), several "Back focus chart" examples can be found on goggle. Since its sole purpose is focus, it is preferred over colored TV test screens. It is crucial the image has small circles on each corner and one in the middle.
Example applications are ORQA Pilot.One goggles, they have this as an option "Deep focus" which is awesome. Some users adjust IPD/focus looking at the camera footage (inaccurate) or their OSD on their camera footage (depending on where your OSD is, inaccurate). A back focusing chart will make sure you get the best sharpness out of your setup.

This is a good idea.

This is a good idea.

Excellent idea, maybe even include the steps needed to dial in focus and IPD.

i have never heard of these before, but does indeed seem like a useful addition. do you think you could provide a 1920x1080 version of this chart? i assume we would want it to fill the whole screen.

Please find attached - with many thanks and credits to Ian from the Houston FPV race community, he made one from scratch. Feel free to suggest any changes/adds.

i have never heard of these before, but does indeed seem like a useful addition. do you think you could provide a 1920x1080 version of this chart? i assume we would want it to fill the whole screen.

Please find attached - with many thanks and credits to Ian from the Houston FPV race community, he made one from scratch. Feel free to suggest any changes/adds.

Awesome, thank you!

i have create a initial implementation for this, you can find binaries to try here https://github.com/hd-zero/hdzero-goggle/actions/runs/3857514237
be advised that this feature builds on some experimental changes, so you want to revert back to a stable release for continued use.

i have create a initial implementation for this, you can find binaries to try here https://github.com/hd-zero/hdzero-goggle/actions/runs/3857514237 be advised that this feature builds on some experimental changes, so you want to revert back to a stable release for continued use.

nice!
Is this firmware based on 122.8?

i have create a initial implementation for this, you can find binaries to try here https://github.com/hd-zero/hdzero-goggle/actions/runs/3857514237 be advised that this feature builds on some experimental changes, so you want to revert back to a stable release for continued use.

nice! Is this firmware based on 122.8 + experimental features?

jup, <IP_ADDRESS> is fully merged.

i have create a initial implementation for this, you can find binaries to try here https://github.com/hd-zero/hdzero-goggle/actions/runs/3857514237 be advised that this feature builds on some experimental changes, so you want to revert back to a stable release for continued use.

seems to be working :)

yall are awesome!

personally i find the white background pretty harsh on the eyes (those oleds are just so bright!)
should we invert the colors? have the background be black and the sections be white?
would that reduce the versatility in any way?

personally i find the white background pretty harsh on the eyes (those oleds are just so bright!) should we invert the colors? have the background be black and the sections be white? would that reduce the versatility in any way?
like this ![dark]

Yeah that's a good idea - be careful with the conversion (above was a bit smudged in the center) or let us know if you need help with that.

Looks like you've got a good inverted copy, but here's an SVG I made if anyone wants to further customize colors:

Just need to edit the source in a text editor, then it should be able to be rasterized by GIMP or similar.

build of the latest version with the inverted screen here https://github.com/hd-zero/hdzero-goggle/actions/runs/3859396827


Added table of contents

Table of contents for 108 fonts with links to their font directory

@diluteoxygen Thank you for your great passion to this project.

@diluteoxygen Hey bro, I've invited you to this project if you like it.

Thank you dude <3


Checking is slow in project consuming xstate types

The original project is here: https://github.com/AlCalzone/node-zwave-js
The actual owner is @AlCalzone

Clone https://github.com/amcasey/ZwavePerfRepro
npm ci
tsc -p .

The portion of the code below // ACASEY boils down to a single function call that takes the vast majority of the checking time.  The portion of the code above is enough scaffolding to make it compile.
I've framed the problem in terms of tsc because it's easier to investigate, but the editor experience is also terrible.

At a high level, the issue is that the many types in the 600-line object literal depend on the parameter type of the function to which it is passed, which depends on the type of the argument (it's not quite a cycle, but it's close).

If you pull out just the on property, that's a pretty good chunk of the check time (but, hopefully, much simpler to analyze).
const myOn = {
        add: [
            // We have control over when pre transmit handshakes are created
            // so we can trigger them immediately without queuing
            {
                cond: "isPreTransmitHandshakeForCurrentTransaction",
                actions: [
                    assign({
                        preTransmitHandshakeTransaction: (_, evt) =>
                            evt.transaction,
                    }),
                    raise("preTransmitHandshake"),
                ],
            },
            {
                actions: [
                    assign({
                        queue: (ctx, evt) => {
                            ctx.queue.add(evt.transaction);
                            return ctx.queue;
                        },
                    }),
                    raise("trigger"),
                ],
            },
        ],
        // The send thread accepts any message as long as the serial API machine is not active.
        // If it is expected it will be forwarded to the correct states. If not, it
        // will be returned with the "unsolicited" event.
        message: [
            {
                cond: "isExpectedUpdate",
                actions: forwardNodeUpdate,
            },
            {
                cond: "isExpectedHandshakeResponse",
                actions: forwardHandshakeResponse,
            },
            {
                actions: (
                    _: any,
                    evt: SerialAPICommandEvent & { message: Message },
                ) => {
                    implementations.notifyUnsolicited(evt.message);
                },
            },
        ],
        // Do the same if the serial API did not handle a message
        serialAPIUnexpected: [
            {
                cond: "isExpectedUpdate",
                actions: forwardNodeUpdate,
            },
            {
                cond: "isExpectedHandshakeResponse",
                actions: forwardHandshakeResponse,
            },
            {
                actions: (
                    _: any,
                    evt: SerialAPICommandEvent & { message: Message },
                ) => {
                    implementations.notifyUnsolicited(evt.message);
                },
            },
        ],
        // Accept external commands to sort the queue
        sortQueue: {
            actions: [sortQueue, raise("trigger")],
        },
    } as TransitionsConfig<SendThreadContext, SendThreadEvent>;

I've pushed a Simplified branch with a greatly reduced repro.  It takes about half as long as the full repro, but a lot of that came out of program time.  The Type and Instantiation counts are still very high (and depend entirely on the declaration of _on, mentioned above).

I'm seeing a slow comparison (roughly half of check time) between two instantiations of State.

I've updated the Simplified branch with this further reduced repro:
import { AssignAction, TransitionConfig } from "xstate";

interface DummyContext {
}

interface Event1 {
    type: "evt1";
}

interface Event2 {
    type: "evt2";
}

declare const myAction: AssignAction<DummyContext, Event1>

const _on: TransitionConfig<DummyContext, Event1 | Event2> = {
    actions: [myAction],
};

(That's it!  You could copy-paste, but cloning conveniently pulls down package-lock.json, etc.)

@davidkpiano It seems strange that Action contains both AssignAction and its base type ActionObject.  Was it intentional and, if so, what was the goal?  I'm pretty sure it's contributing to the perf issues some consumers are seeing (because the compiler has to compare two very similar types).
p.s.  Please let me know if you'd rather have an issue in the xstate repo - I started here to spare you the clutter.

@amcasey I believe that was part of the effort to get inference right for something like this:
{
  actions: assign({/* ... */})
}

Where we wanted that to "just work" without having to specify generic types for assign every time, e.g.,:
{
  actions: assign<SomeContext, SomeEvent>({/* ... */})
}

Hence actions being typed as:
type Action =
  | ...
  | AssignAction<...>
  | ...

For proper inference. I will look into simplifying that type.

@davidkpiano I'm not super-familiar with xstate, but the customer repro I'm working with produces the same completion items in that position if I delete either AssignAction or ActionObject (just much more quickly).  It looks like they're just based on the context type argument, which should be inferred the same, regardless of which is removed.

I believe @davidkpiano's change resolved this issue.  Thanks!
p.s.  I'm still investigating how we could have made the problem easier to identify.

Thanks everyone, I will test it out after my vacation 👍🏻

@davidkpiano @amcasey
Just did a quick test using today's TypeScript and xstate from https://github.com/davidkpiano/xstate/issues/1350#issuecomment-668852857.
The original repro I've sent @amcasey via mail now takes 8s on my NAS instead of over 30. On my desktop, we're talking 1-2 seconds instead of 5. Although I'm not sure how much of that is the request for completions.

Nice, that's quite the improvement. Please let me know if there are other optimizations I can make in XState and I will prioritize those.

@AlCalzone it should be about 50/50 completion and squiggles - they're doing the same work (separate discussion :|).
I think I see another hotspot and I'm investigating now.

@davidkpiano Does this intentionally not pass through the fourth type argument (TTypestate)?
https://github.com/davidkpiano/xstate/blob/ab7329eaf849b613b94cb815a7e1575e9c9f7ea9/packages/core/src/StateNode.ts#L222

@amcasey It's not intentional; it should.

@davidkpiano FWIW, I don't think that's the problem - I just noticed it as I was digging.

@AlCalzone as a mitigation, you can add this type annotation:
const incrementSendDataAttempts: AssignAction<SendThreadContext, SendThreadEvent>

@davidkpiano Changing TEvent extends { type: K } ? TEvent : never to TEvent at https://github.com/davidkpiano/xstate/blob/ab7329eaf849b613b94cb815a7e1575e9c9f7ea9/packages/core/src/types.ts#L340 saves me another ~10% of check time.  I kind of wanted to ask about that condition anyway, since it looks like it just rules it event types that are unions of string literals.  Assuming you're happy with the change, it should probably apply to the Array variant as well (smaller benefit, since less consumed).

I kind of wanted to ask about that condition anyway, since it looks like it just rules it event types that are unions of string literals.

The purpose for that rule is that we want to limit what kinds of events are in the transitions:
type SomeEvents =
  | { type: 'count'; value: number; }
  | { type: 'foo' };

// ...
on: {
  count: {
    //       inferred: { type: 'count'; value: number; }
    //                 vvvvv
    actions: (context, event) => {
      console.log(event.value);
    }
  }
}

Is there a more performant way to do this? Changing that to TEvent won't provide that safety.

Sorry, I'm not sufficiently familiar with xstate to understand your example.  What's being ruled out?  Also, how's it being ruled out?  Does the never make the resulting object unusable or something?
I'll also point out that there's a value judgement to be made here (which I don't have enough information to do): if this code is representative and the more precise type adds 10% to check time for a large percentage of users, is it still worthwhile?  If it catches a common or subtle bug, then it probably is.  If it catches something rare/hypothetical or something that will be caught by the first test, then a faster build/editor might be preferable.
Anecdotally, the project motivating this issue was hitting this case (i.e. the event type became never) and it was thoroughly non-obvious how to fix it (as I recall, inference for the whole state machine object failed and the invalid event type wasn't flagged in any way).

It catches a common bug. Expanding on the above example:
type SomeEvents =
  | { type: "COUNT"; value: number }
  | { type: "MESSAGE"; value: string };

interface SomeContext {
  count: number;
}

const machine = createMachine<SomeContext, SomeEvents>({
  context: { count: 3 },
  // ...
  on: {
    MESSAGE: {
      actions: (context, event) => {
        console.log(context.count * event.value);
      }
    }
  }
});

The event is { type: 'MESSAGE'; value: string; } and properly inferred here; whereas if it were SomeEvents, the developer would have to specify event as SomeEvents & { type: 'MESSAGE' } (or something similar) instead to get proper inference.

Unfortunately I dont currently have a spare minute to help with this issue but I just want to throw in my words of encouragement.
This is really important work for all us TS users and im glad you guys @amcasey and @davidkpiano are working on it. It sadly sounds like theres going to be some tradeoffs of performance vs type safety here.
Im also wondering about regressions as new versions of XState are released. Would it be worth having some sort of "TS Performance Test Suite" that runs on the CI?

Im also wondering about regressions as new versions of XState are released. Would it be worth having some sort of "TS Performance Test Suite" that runs on the CI?

That would be awesome. New versions of XState will be largely compatible with the current version, and typings will be similar. @mattpocock and @Andarist are also working on a TypeGen which will assist greatly with typing machines without compromising performance.

@davidkpiano I get the same error when I use this (faster) declaration:
declare type TransitionsConfigMap<TContext, TEvent extends EventObject> = {
    [K in TEvent['type'] | '' | '*']?: TransitionConfigOrTarget<TContext, TEvent>;
};

I'm pretty sure it will only make a difference when you have events like:
type SomeEvents =
  | { type: 'count' | 'offset'; value: number; }
  | { type: 'foo' };

I've prepared a simplified playground showcasing why this matters - look at the declared config and the cond property in it.
The current version (working):
https://www.typescriptlang.org/play?#code/C4TwDgpgBAyglgOwOYBsIHkBOBBTmCGIAPACoB8UAvFCQNoC6UAPjQNwBQ7iwEmAZvgDG0AKIA3CAmDoARgCsIg4FADe7KFFCQAXFADOwTIiQcAvpy3QSBBHrjA4Ae1sBhZ3zhIAsvjCk3UhAAHsAANDTiksrBPAgAJnpQkVKyCkoU1GoatADSUIgRElK0AESWJfT0APy61vi29k4IAR5IWCT4mEgQwETqGjQBPCGh-RokydEhkgmqmuAQunmmUFWFUVC6CBASmP1kZhzsljQ2dg7OLZ4dXT1U+obGzFAArvEQHttxR9y8AsKnernJpXJD+ZzDMLrKRQGIzRKTVKKYAULJQQTOOI1KAACiUQVqQxi4R2tUmAEoqBQZI5HGh6hwNMBOt1gLUzo1Lu5riyemYLAtAQ0Ls1uW1MDdWX1xkSRv0JkUprFZoj5Mj2BlYMY0FhcARiHVhSCxZK7ixDcCuQhWuDAiNoSiDgLIFBQbbIeEFRs4fEEYqkel7mjnNiLZzXGKfH4SLKoV6pE7TEcMbZlII+EhdG7+iooHF8MzdCUALaOOIQFAlKCmUYaXOWIsAMXQ6BKrCgYEwjjAWxexZkvGrz3rCyLACFsAAlNsdrs9ubbAwQOK6AxGZBD8ya4MIXRojQTyd7sYDZm3NlQEoyfAAL2M+BKtYGGhTK9xAH18SSxJTKKiT8+mA9C8mAILCYgAHSdt2EGLjwcRUJQ1AlECADuvBtgB1ZPhoNb9HhSbsEAA
Proposed version (not working):
https://www.typescriptlang.org/play?#code/C4TwDgpgBAyglgOwOYBsIHkBOBBTmCGIAPACoB8UAvFCQNoC6UAPjQNwBQ7iwEmAZvgDG0AKIA3CAmDoARgCsIg4FADe7KFFCQAXFADOwTIiQcAvpy3QSBBHrjA4Ae1sBhZ3zhIAsvjCk3UhAAHsAANDTiksrBPAgAJnpQkVKyCkoU1GoatADSUIgRElK0AESWJfT0APy61vi29k4IAR5IWCT4mEgQwP7OPCHhJMnAZGYc7JY0NnYOzi2eHV09VPqGxsxQAK7xEB4IEHET3LwCwtP1s00LSH2Bg4VRUDGSCUlF0vKKo6rqUILOOI1KAACiUQVqAQGYWeYlqIwAlFQKDJHI40PUOBpgJ1usBajNGvN3ItcT0zBZwFZCXNmiS2pglniiH8SFCYqFWSNniFXokRqlvuwMrBjGgsLgCMQ6g1aTcmSsWDKrsSEK07tChiMyGNKZAoDcNRzHlIebE3gKvulVlkoM5gcqia56T4-Gz+sbhh9daYJgDbMpBHwkLpDX8VFA4vgcboSgBbRxxCAoEpQUycjQRyyxgBi6HQJVYUDAmEcYF0CC2cZkvDTmyzVNjACFsAAlQvF0vl1RQA4GQ66AxGZB18wi23OXS2jQt1tTv4abFk-FQEoyfAAL2M+BKGcXGn9cV0IIA+uDwhAxEjKBRp-uoJgeltMAhYQA6Etlt99nhxKiUagSkuAB3XhCwXRd0wgqCNCg312CAA

Thanks, @Andarist, examples like that are very helpful!  I always forget that conditionals distribute across unions.  If that's what you're going for, IMHO (as someone who totally failed to understand this, so grain of salt...) is that it would be more straightforward to have the conditional outside the generic type: TEvent extends { type: K } ? TransitionConfigOrTarget<TContext, TEvent> : never.
I'll ask around in case there's a more direct way to express that.

Purely for your amusement, I thought this might help, but it makes checking 4X as slow. 😆
TransitionConfigOrTarget<TContext, TEvent & { type: K }>

I think I've convinced myself that this fragment of TransitionsConfigArray
{
    [K in TEvent['type']]: TransitionConfig<TContext, TEvent extends {
        type: K;
    } ? TEvent : never> & {
        event: K;
    };
}[TEvent['type']]

is trying to form the union of TransitionConfig<TContext, K_part_of_TEvent> & { event: K } for each type of event, K.  If that's the case, I think I've convinced myself that we can use more conditional-distribution to write it as
TEvent extends EventObject ? TransitionConfig<TContext, TEvent> & { event: TEvent["kind"]; } : never

which is noticeably faster (10-20%) on my box.
Note that I'm not recommending this as a general pattern - I only suggest sneaky perf changes in code that has been measured to be slow.

I believe a bunch of time is going into distinguishing TransitionsConfigMap from TransitionsConfigArray.  I'm still looking for a way to speed up the check.

If I am correct in believing TransitionsConfigMap will never have an "event" property, you can prepend {event?:never} & to the beginning of the existing union.  On my box, it's a substantial speedup.
To summarize, I now have
declare type TransitionsConfigMap<TContext, TEvent extends EventObject> =
    { event?:never }
    & { [K in TEvent['type']]?: TEvent extends { type: K; } ? TransitionConfigOrTarget<TContext, TEvent> : never; }
    & {
        ''?: TransitionConfigOrTarget<TContext, TEvent>;
        '*'?: TransitionConfigOrTarget<TContext, TEvent>;
    };
declare type TransitionsConfigArray<TContext, TEvent extends EventObject> = Array<
    TEvent extends EventObject ? TransitionConfig<TContext, TEvent> & { event: TEvent["type"]; } : never
    | (TransitionConfig<TContext, TEvent> & { event: ''; })
    | (TransitionConfig<TContext, TEvent> & { event: '*'; })>;

Sorry for mucking with your line wrapping. :smile:

In the original (i.e. unreduced) repro, I'm seeing
Before
Types:                       59271
Instantiations:             218383
Check time:                  4.41s

After
Types:                       42758
Instantiations:             149793
Check time:                  3.50s

I'm not sure if there is any recommendation for us here now 😅 Some of your posts seem to refer to the next ones? Or I have just misunderstood your intention. Could you provide a final version of your recommendation? I could then try to assess if it's correct from our POV.
Also a note - I'm also a little bit surprised that this (what we currently have) works:
type TransitionsConfigMap<TContext, TEvent extends EventObject> = {
  [K in TEvent["type"]]?: TransitionConfigOrTarget<
    TContext,
    TEvent extends { type: K } ? TEvent : never
  >;
};

Like - it's not super clear to me that TEvent gets narrowed down by this conditional.
Something like:
TEvent & { type: K }

or
Extract<TEvent, { type: K }>

would be more clear to me. Are you aware of any perf characteristics of all of those options?

@Andarist sorry, my investigation notes tend to be pretty stream-of-consciousness. :smile:  I tried to indicate the final state in my "to summarize" comment and the numbers reflect the performance impact of those changes.
Having said that, I'd hold off a bit longer because we can't actually figure out why that change helps so much (or just drop { event?: never } - the rest of the change seems sound).
Regarding your current code, it took me a while to figure that out too, because I always forget that conditionals distribute over unions.  My mental model (which probably isn't precisely correct) is that T1 | T2 extends TCond ? TTrue : TFalse is treated as (T1 extends TCond ? TTrue : TFalse) | (T2 extends TCond ? TTrue : TFalse).
I did try TEvent & { type: K } since that seemed most natural to me too, but it caused a huge slowdown.  I'm planning to isolate that and file a fresh bug (against us, not you).
I would agree that it's generally best to avoid intersection types as much as possible.

@amcasey Do you know if the changes in Typescript that helped speed up my project are included in 4.0.2?

Finding #1: @AlCalzone, you (and other consumers of xstate) probably want "strictFunctionTypes": true, because there are so many variance checks when using xstate types.  Forcing the variance checks will probably also speed things up (because they're cheaper than structural checks).

Finding # 2: It sure looks like something  is going wrong.  At the very least, the behavior is surprising.  (Note that the issue is about correctness, rather than perf, but I conjecture that the behavior change is resulting in short-circuiting of expensive logic, affecting the perf.)

probably want "strictFunctionTypes": true, because there are so many variance checks when using xstate types

I think I'll pass 😨

Aside from that, this breaks a bunch of typed function declarations (or at least makes them much less elegant).

@AlCalzone I agree that that error text is intimidating, but - in the absence of evidence to the contrary - I'm inclined to believe that it indicates a real problem with the code.
Having said that, if you feel it's making your code worse, I'd be interested to know more.

Having said that, if you feel it's making your code worse, I'd be interested to know more.

Nevermind, I was a bit tired yesterday. I guess strictFunctionTypes did what its supposed to do.
I have a function signature type with two generics, both with a constraint and default types. However the generics weren't actually used, so it turns out I misused them for assignability checks.
So instead of
type CCResponsePredicate<
	TSent extends CommandClass = CommandClass,
	TReceived extends CommandClass = CommandClass
> = (sentCommand: TSent, receivedCommand: TReceived) => CCResponseRole;

...

const testResponseForSoundSwitchToneInfoGet: CCResponsePredicate = (
	sent: SoundSwitchCCToneInfoGet,
	received: SoundSwitchCCToneInfoReport
) => ...

I can just do
type CCResponsePredicate = (sentCommand: CommandClass, receivedCommand: CommandClass) => CCResponseRole;

const testResponseForSoundSwitchToneInfoGet: CCResponsePredicate = (
	sent: SoundSwitchCCToneInfoGet,
	received: SoundSwitchCCToneInfoReport
) => ...

and get the same errors if I mistype something in the implementation signature.

I'm inclined to believe that it indicates a real problem with the code.

Do you mean my code or xstate's types?

I'd be interested to know more.

I'm pretty sure this is intended behavior, but it makes enforcing assignability constraints cumbersome.
Basically I need to enforce that the parameters of predicates passed to a function are subclasses of the predicate argument types:
https://www.typescriptlang.org/play?strictFunctionTypes=false&noImplicitReturns=false#code/CYUwxgNghgTiAEkoGdnwLIlVA5gg3vAA4wCWAblAC4IC2AjAFzwCuAdgNZsD2A7mwG54AXwCwAKFBI4iaKngBlIuFJQImbHnggAHjTbA0G5LgLEylGvFoAmZuy59BIiVOgyk8gHLcqAQWNTeEISCmo6AGZ7Th5+ITFxCSoAT2VzEGBSMHCAHgAVbT0QAyMsEy0AXgwy0wA+eCqAClpkHGY8gEoG+oAjbm4IEChBV3B3BAAzdjAqUm42eAn+xqJmEgys8I7mPoGhkcTxAHoj+F5SKgALbhYqeGQqMhmAMWnZ+bzUrHgr0jQ-+DcDgSMDzB7EehQNZwTLZKxNFptRTKMCqdQ1PBdCr1fASeD4+BwKgsGALR4sEACCTCKniJbcFaQjq0iQnM4Xa63e6PLJUV5sGZzNifZRoX7-NCgmBwGYQZLwUA0GYZeAoVULEDS7gwVmnHz+QJaHh3FDIUg4NhQHqDH7caqaEAgsF3Ij0HrQjZwhAI1rMfUBDEgLE4vEEokkskwCm0mkSemMnrMoA
With strictFunctionTypes, the generics must be passed through and are decoupled from the implementation params:
https://www.typescriptlang.org/play?noImplicitReturns=false#code/CYUwxgNghgTiAEkoGdnwLIlVA5gg3vAA4wCWAblAC4IC2AjAFzwCuAdgNZsD2A7mwG54AXwCwAKFBI4iaKngBlIuFJQImbHnggAHjTbA0G5LgLEylGvFoAmZuy59BIiVOgyk8gHLcqAQWNTeEISCmo6AGZ7Th5+ITFxCSoAT2VzEGBSMHCAHgAVbT0QAyMsEy0AXgwy0wA+eCqAClpkHGY8gEoG+oAjbm4IEChBCQB6UfheUioAC3hkKjIwKgAxdmXSbjY81Kx4AEl4NhAM+CpuYhQ0PGOltFmYbhYcOZByEBhk3hmPkFdwdwIABm6yomzY8CB-XyhX0hmqmhAtUaRGYJAyWVyeVqHWYfQGQxGiXE43gw2AZx+yWsLAW8B6CGQyjApCBpFOpAhswQKTSblg1HBEjAWzpRBsUDRcEy2RoOSUKjUgTw9SaLRwXQq9XwEngevgcCoLBgXJgLBAAgkwkt4ih3BREo6NrGExFMDgywg1NANGWpxQZIhH0eMGFoqoxBsPSlGNlIByPn8yqRDXgzVamu1uv1huNpvNNutEjtDp6TqAA

@amcasey with a bunch of changes I could build the project again. However there seems to be a problem with raise actions now (which are the source of the wall of errors).
You can play with this tree: https://github.com/AlCalzone/node-zwave-js/tree/e10cb2c6d706bfc9dda0b5039ec38c651a2fbbd7
In https://github.com/AlCalzone/node-zwave-js/blob/e10cb2c6d706bfc9dda0b5039ec38c651a2fbbd7/packages/zwave-js/src/lib/driver/SendThreadMachine.ts, you'll see I had to any some of the raise actions, e.g. line 394 or 405.
I'm not sure why these would cause an issue. @davidkpiano I assume those should just work?

@AlCalzone I've forked your second playground here, you can look at the playground or read through it here as I'm going to basically copy-paste stuff here
My comments are purely a comparison to your first playground - I have discarded other examples that you have included in the second one but not in the first one.
You have included this:
// and they must be specified in the type declaration
const p2a: predicate<SpecialMessage> = (msg) => {
    return true;
};
foo(p2a);

but if I remove the explicit type param this works OK, so I'm not sure what you are referring to when saying that "they must be specified in the type declaration".
case1: "without strictFunctionTypes this is ok" from the other playground
const p1a: predicate = (msg: SpecialMessage) => {
    return true;
};
foo(p1a);

const p2a: predicate<Message> = (msg: SpecialMessage) => {
    return true;
};
foo(p2a)

Those are both errors, but why exactly? Let's look at an example that is somewhat easier to understand:
class Animal {
    walk() {}
}
class Dog extends Animal {
    bark() {}
}
function acceptDog(dog: Dog) {
  dog.bark()
}
const acceptAnimal: (anim: Animal) => void = acceptDog
acceptDog(new Animal()) // ERROR!

This makes total sense because acceptDog could use dog-specific properties (and it does - it calls .bark()) and an instance of Dog's super-class just doesn't satisfy this constraint. Tt's basically what happens here, this is 100% correct, maybe you have already known this but I felt it might be worth reiterating through this example to demonstrate that this behavior is preventing bugs.
case2: without strictFunctionTypes this is correctly detected as an error
const p1b: predicate = (msg: NotAMessage) => {
    return true;
};
foo(p1b);

This one is still detected as an error, even without an explicit type param
raise issue
Could you raise an issue about this in XState? I haven't looked into this yet at all - so maybe it's just working as intended but maybe it is not. It will be easier for us to track it when reported in our issue tracker

but if I remove the explicit type param this works OK, so I'm not sure what you are referring to when saying that "they must be specified in the type declaration".

Maybe I was unclear. These predicate functions are taylored to specific subclasses of the Message class, so I need to specify the type somewhere. Doing it for the function argument improves readability IMO.

This makes total sense because acceptDog could use dog-specific properties

True, I haven't thought about it this way. I guess my use case aims a lot lower. Here's an example:
function testResponseForBinarySensorGet(sent: BinarySensorCCGet, received: BinarySensorCCReport) {...}

@expectedCCResponse(BinarySensorCCReport, testResponseForBinarySensorGet)
export class BinarySensorCCGet extends BinarySensorCC { ... }

The logic that uses this decorator internally makes sure that the first predicate argument is an instance of the class the decorator is on (BinarySensorCCGet). The decorator itself makes sure that the second predicate argument is an instance of the first decorator argument (BinarySensorCCReport).
So basically the check was supposed to check if the predicate argument types are both subclasses of the common super class of all these CC classes. If not, I probably messed up while auto-importing something (it has happened before!).

@AlCalzone

by the way, intellisense no longer has any noticeable delays now :)

Excellent!

But there's still a discrepancy between lerna run build, npm run watch and the language server in reported errors.

https://github.com/microsoft/TypeScript/issues/39811

I'm pretty sure this is intended behavior, but it makes enforcing assignability constraints cumbersome.

Variance is incredibly confusion, but I think I might see what's going wrong in your examples.  (This is basically just a rehash of @Andarist's excellent explanation, but maybe another framing will be helpful.)
Suppose I have an Animal[].  I can put an instance of a subtype of Animal, e.g. Dog, in the array and everything is fine, because a Dog is an Animal and any sort of Animal can go in the array.
Now, suppose I have a dog-watcher - a Watcher<Dog>.  Obviously, my dog-watcher can watch any sort of dog.  If I ask it to watch a cat, no good.  What happens if all the dog-watchers I know are sick and I need someone else to watch my dog.  In this case, we have the opposite behavior from an array - to replace a Watcher<Dog>, I need someone who can watch a supertype of Dog - maybe a Watcher<Animal>, because someone who can watch any animal can certainly watch any dog.  (What if I went the other way?  I might end up with a chihuahua-watcher for my pitbull and everyone would be unhappy.)
A predicate is more like a watcher than an array.  When I say I'm willing to accept any function that validates Dogs (i.e. predicate<Dog>), I mean that I would accept any function that is at least capable of validating any Dog (e.g. predicate<Animal>, which can validate any animal).  Having the type parameter in an input position (i.e. as the type of the predicate's parameter) means that I want to accept supertypes, rather than subtypes.
So, as far as I can tell, the compiler is correctly detecting that your expectation was backwards - I can assign a predicate<Message> to a predicate<SpecialMessage> because it's more general, but not the other way around (otherwise, someone might test a SpecialMessage2 with the predicate and it wouldn't know how to handle it.)
Unfortunately, there's really no way we could fit an explanation of variance into a compiler error message. 😢

you'll see I had to any some of the raise actions, e.g. line 394 or 405.

I'm not sure what's going on with raise (since I haven't played with your branch yet), but it sounds like you're going to discuss it on the xstate repro.  Obviously, feel free to bring it back here or loop me in if the compiler appears to be behaving incorrectly. :smile:


Method to toggle Pattern Playback Follow on/off

I would prefer to be able to select if Pattern Playback Follow is on or not -- so I can continue tracking even with the song playing.

If you move your cursor up and down, it ceases to follow the play position. Is this enough? Again, we could make a button to toggle "follow on/off" but this would be yet another toggle in the GUI to remember to click/unclick.
How about just ctrl-Space or something like that to start playing without the note tracking on?

I pushed Ctrl-Space and F5-F7 key commands to play different ways. Ctrl-Space starts without notetracking; F5 plays from current pos & notetracking on, F6 plays from current pos with notetracking on and F7 continues wherever it was when it last stopped.

thanks, this solves a lot. closing.


Detect and deploy AMD GPUs

Is your feature request related to a problem? Please describe.
Omnia should be able to detect and deploy AMD Radeon GPUs
Describe the solution you'd like
Installation instructions for ROCm: https://rocmdocs.amd.com/en/latest/Installation_Guide/Installation-Guide.html#installing-centos-v7-8-v8-2-for-dkms
Describe alternatives you've considered
N/A
Additional context
N/A

We will do this through #108.

Reopening this issue to track AMD enablement.

Reopening this issue to track AMD enablement.

Adding link to AMD GPU device plugin for Kubernetes: https://github.com/RadeonOpenCompute/k8s-device-plugin

Adding link to AMD GPU device plugin for Kubernetes: https://github.com/RadeonOpenCompute/k8s-device-plugin


fastlane crashes with gem conflcit

Just updated to fastlane 0.1.19 and running it I got:
$ fastlane prod --env monits
/System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/specification.rb:2007:in `raise_if_conflicts': Unable to activate frameit-0.2.0, because commander-4.3.0 conflicts with commander (~> 4.2.0) (Gem::LoadError)
	from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/specification.rb:1176:in `activate'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/specification.rb:1212:in `block in activate_dependencies'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/specification.rb:1198:in `each'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/specification.rb:1198:in `activate_dependencies'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/specification.rb:1180:in `activate'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/core_ext/kernel_gem.rb:48:in `gem'
	from /usr/bin/fastlane:22:in `<main>'

Turns out gem update fastlane doesn't update all the tools it depends on. Updating everything got it working.

That's weird, it should update the other tools. What tool didn't update? sigh or cert? The dependency I added should make use of the latest version: https://github.com/KrauseFx/fastlane/blob/master/fastlane.gemspec#L36
Do you know what's wrong?

I got that error with frameit, snapshot and pem. I then got tired and simply updated every tool in the fastlane family.

Thanks, I hope to get everything fixed with the next release :+1:

I get the same error
/System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/specification.rb:2007:in `raise_if_conflicts': Unable to activate fastlane_core-0.46.3, because commander-4.4.0 conflicts with commander (= 4.3.5) (Gem::LoadError)
	from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/specification.rb:1176:in `activate'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/specification.rb:1212:in `block in activate_dependencies'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/specification.rb:1198:in `each'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/specification.rb:1198:in `activate_dependencies'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/specification.rb:1180:in `activate'
	from /System/Library/Frameworks/Ruby.framework/Versions/2.0/usr/lib/ruby/2.0.0/rubygems/core_ext/kernel_gem.rb:48:in `gem'
	from /usr/local/bin/fastlane:22:in `<main>'

after running gem cleanup... after upgrading to the latest fastlane.

@powtac can you try again, we just pushed a fix 👍

After running sudo gem update fastlane is working again.

@powtac  Awesome! Really glad to hear you are up and running. Hope you have an excellent week! 👍


Stabalize tests by seting all android sdk path environment variables

Fixes #12300

Tests would fail if ANDROID_HOME or ANDROID_SDK were also set
Tests now make sure to set, clear, and test all of the possible values (ANDROID_HOME, ANDROID_SDK_ROOT, ANDROID_SDK)

Can this be generalized in a before_each?


libsodium 1.0.6

Fixed exception messages in PublicKeyBox
Updated the libsodium dll's to 1.0.6
Added sodium_increment
Added sodium_compare
Added crypto_aead_aes256gcm_*

Fixes #119

we need to update the libsodium dll before travis will work

Hi @adamcaudill can you please verify this?

the commit already includes the 1.0.6 dlls

@bitbeans can you verify if your tests runs fine?
Travis failed with
5) Test Failure : Tests.VersionTest.SodiumVersionStringTest

     String lengths are both 5. Strings differ at index 4.

  Expected: "1.0.6"

  But was:  "1.0.3"

I believe you must also change the build scripts in travis to account for libsodium 1.0.6

The version of libsodium needs to be updated in travis-build-libsodium.sh - then it looks like it's good to merge.

Well, this is unexpected. Tests are fine on Linux, but fail on OSX - with a rather unusual error:
Tests run: 192, Errors: 2, Failures: 0, Inconclusive: 0, Time: 17.598771 seconds
  Not run: 0, Invalid: 0, Ignored: 0, Skipped: 0

Errors and Failures:
1) Test Error : Tests.SecretAeadAesTest.AesAeadWithAdditionalDataTest
   System.ExecutionEngineException : SIGILL
  at (wrapper managed-to-native) DynamicDllInvokeType:crypto_aead_aes256gcm_encrypt (intptr,long&,byte[],long,byte[],long,byte[],byte[],byte[])
  at (wrapper delegate-invoke) <Module>:invoke_int__this___intptr_long&_byte[]_long_byte[]_long_byte[]_byte[]_byte[] (intptr,long&,byte[],long,byte[],long,byte[],byte[],byte[])
  at Sodium.SecretAeadAes.Encrypt (System.Byte[] message, System.Byte[] nonce, System.Byte[] key, System.Byte[] additionalData) [0x00000] in <filename unknown>:0
  at Tests.SecretAeadAesTest.AesAeadWithAdditionalDataTest () [0x00000] in <filename unknown>:0
  at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&)
  at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00000] in <filename unknown>:0

2) Test Error : Tests.SecretAeadAesTest.AesAeadWithoutAdditionalDataTest
   System.ExecutionEngineException : SIGILL
  at (wrapper managed-to-native) DynamicDllInvokeType:crypto_aead_aes256gcm_encrypt (intptr,long&,byte[],long,byte[],long,byte[],byte[],byte[])
  at (wrapper delegate-invoke) <Module>:invoke_int__this___intptr_long&_byte[]_long_byte[]_long_byte[]_byte[]_byte[] (intptr,long&,byte[],long,byte[],long,byte[],byte[],byte[])
  at Sodium.SecretAeadAes.Encrypt (System.Byte[] message, System.Byte[] nonce, System.Byte[] key, System.Byte[] additionalData) [0x00000] in <filename unknown>:0
  at Tests.SecretAeadAesTest.AesAeadWithoutAdditionalDataTest () [0x00000] in <filename unknown>:0
  at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&)
  at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00000] in <filename unknown>:0

There seems to be a large number of issues that can cause this error - but it's odd that it's only happening on OSX.

Glad you're back @adamcaudill :)
Yeah that is why I thought lets first update libsodium and double check if tests pass.
I will also test here but for now someone with osx to test?

I can`t test OSX :(

Is it actually OSX, or the fact that it was tested on a Mac with an old CPU?
aes256gcm is only available on CPUs with hardware-accelerated AES. On older CPUs, the instruction doesn't exist, so you get SIGILL.
crypto_aead_aes256gcm_is_available() must be called to check whether aes256gcm is available or not.

crypto_aead_aes256gcm_is_available is failing on my local system with the current stable release 1.0.6. But with the master (1.0.7) it just runs fine.

@jedisct1 is there already a release date for 1.0.7?

Very soon!
Meanwhile, you can check out the stable branch.

thanks @jedisct1 @bitbeans looks good :)

@jedisct1 very soon == tomorrow :+1:

@adamcaudill @fraga can you check the pull request again please?

Looks good for me

Thanks! Sorry it took so long to get this one done.


Skip dataset verifications by default

Skip the dataset verifications (split and checksum verifications, duplicate keys check) by default unless a dataset is being tested (datasets-cli test/run_beam). The main goal is to avoid running the checksum check in the default case due to how expensive it can be for large datasets.
PS: Maybe we should deprecate ignore_verifications, which is True now by default, and give it a different name?

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

100% agree that the checksum verification is overkill and not super useful. But I think this PR would also disable the check on num_examples no ?
As a user I would like to know if the dataset I'm loading changed significantly.
And I also think it can be useful to make sure the metadata are up to date.
What do you think ?
We could have a default ignore_verifications="ignore_checksums"

We could have a default ignore_verifications="ignore_checksums"

Accepting multiple types (booleans and strings) at the same time is not the best design. Maybe we could define an enum for this parameter?

so we can have three verification levels, - smth like "ignore_all" (to skip both checksums and all other info like num_examples verification), "ignore_checksums" (to skip only checksums verification), and "verify_all" (to perform all verification)?
and deprecate ignore_verifications param.
@mariosasko if you're not going to work on this PR in the coming days, I can take over it if you want (this PR will help me with this issue, not super urgent though).

Okay, I propose deprecating ignore_verifications in favor of verification_mode (load_dataset already has download_mode; some other projects use this name for verification control). verification_mode would accept the following enum (or strings in the same manner as download_mode does):
class VerificationMode(enum.Enum):
    FULL = "full"           # runs all verification checks
    BASIC = "basic"     # default, runs only the cheap ones (skips the checksum check)
    NONE = "none"      # skips all the checks

WDTY?

(copy paste from my message on slack)
What do you think of a config variable in config.py to switch from one verification mode to another ? This way we don’t deprecate anything
Many users are familiar with ignore_verifications=True, it might be overkill to deprecate it

@lhoestq So we have "basic" verification mode in config.py and continue to have False as a default
value for ignore_verifications? That way running all verifications including checksums would not be possible without switching the config var, right?
I like having a VerificationMode enum because it's aligned with DownloadMode and sounds more natural to me (ignore_verifications feels a bit semantically reverted but this is probably just my feeling) and it's flexible (no need to worry about config.py, I'm not sure that users even know it exists, wdyt?).
The usage point seems also valid to me, but cases when users are stuck with NonMatchingX errors also happen from time to time (I checked our slack and forum).
As a note aside - I suggest to add instructions to the NonMatchingX error message (how to use ignore_verifications / verification_mode), this would save users who don't know about this param a lot of time.

Ok I see. I'm fine with the new parameter then (even though I had a small pref for the config variable) :)

I like the idea of an enum and the verification_mode parameter.
In relation with the config parameter, we could additionally add a DEFAULT_VERIFICATION_MODE, maybe only if users require it. Note that until now there wasn't any config parameter for a default ignore_verifications value: I guess people are explicitly passing ignore_verifications=True...
As a note aside, I like the suggestion by @polinaeterna: we could give actionable messages when verifying checksums. This could be done in other PR.

This is ready for review.
If verification_mode is None, it defaults to VerificationMode.BASIC instead of VerificationMode.NONE, so maybe we should find a better name for the latter to avoid confusion.
PS: ignore_verifications is still present in the test/run_beam commands for simplicity. Let me know if you think these commands should support all three modes.

I would also prefer to change the name for the NONE verification mode, but don't have really good ideas in mind. maybe smth like SKIP_ALL ?

I decided to go with the following names:

no_checks (previously none)
basic_checks (previously basic)
all_checks (previously full)


dpdk: build with meson, odp-dpdk: -> <IP_ADDRESS>

This converts the dpdk build to meson, and bumps odp-dpdk to the latest version.
Due to some cleanups, we can now build dynamically, and also build on aarch64-linux, which is a very interesting target for dpdk :-)
Things done

[x] Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
Built on platform(s)

[x] NixOS
[ ] macOS
[ ] other Linux distributions

[ ] Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
[ ] Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
[x] Tested execution of all binary files (usually in ./result/bin/)
[ ] Determined the impact on package closure size (by running nix path-info -S before and after)
[ ] Ensured that relevant documentation is up to date
[x] Fits CONTRIBUTING.md.

@GrahamcOfBorg build dpdk odp-dpdk

rebased on top of #73555 (cc @risicle)


Woocommerce 3.0.4 unable to pair

Doesn't work in Woocommerce 3.0.4, Unable to pair with BitPay.

Works for me on Woocommerce 3.
Need more info to find out whether it's a library issue, or a configuration/BitPay account issue.


Calendar Skill - "between 2 and 3" not being used when booking a meeting

Project
Calendar Skills
Description
When booking a meeting with a time range, e.g. between 2 and 3 the LUIS model correctly extracts a datetimerange but the calendar skill doesn't use it and goes on to prompt for this.
To Reproduce

"book a meeting with toby between 2 and 3 tomorrow"

Expected behavior

A datetime of tomorrow and a start of 2pm and a endtime of 3pm

Screenshots

Additional context
{
  "luisResponse": {
    "alteredQuery": null,
    "compositeEntities": null,
    "entities": [
      {
        "endIndex": 48,
        "entity": "between 2 and 3 tomorrow",
        "resolution": {
          "values": [
            {
              "end": "2019-02-08 03:00:00",
              "start": "2019-02-08 02:00:00",
              "timex": "(2019-02-08T02,2019-02-08T03,PT1H)",
              "type": "datetimerange"
            },
            {
              "end": "2019-02-08 15:00:00",
              "start": "2019-02-08 14:00:00",
              "timex": "(2019-02-08T14,2019-02-08T15,PT1H)",
              "type": "datetimerange"
            }
          ]
        },
        "startIndex": 25,
        "type": "builtin.datetimeV2.datetimerange"
      },
      {
        "endIndex": 33,
        "entity": "2",
        "resolution": {
          "subtype": "integer",
          "value": "2"
        },
        "startIndex": 33,
        "type": "builtin.number"
      },
      {
        "endIndex": 39,
        "entity": "3",
        "resolution": {
          "subtype": "integer",
          "value": "3"
        },
        "startIndex": 39,
        "type": "builtin.number"
      }
    ],
    "intents": null,
    "query": "book a meeting with toby between 2 and 3 tomorrow",
    }
  }
}

fixed the exact number can't be recognized issue. but in this case the "3" can't be labelled as ToTime in LUIS. will fix in next version of LUIS model.

@KayMKM Did this get fixed with the latest LUIS models from the pre-built team?

Closing, because this seems have been resolved.


Patch release (mime issue)

@JuliaRegistrator register()

Registration pull request created: JuliaRegistries/General/505
After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.
This will be done automatically if Julia TagBot is installed, or can be done manually through the github interface, or via:
git tag -a v0.1.1 -m "<description of version>" 4687a9b25a3b44cdd30bc3d129caf4d6ce757cd7
git push origin v0.1.1

Adding compat bounds
@JuliaRegistrator register()

Registration pull request updated: JuliaRegistries/General/505
After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.
This will be done automatically if Julia TagBot is installed, or can be done manually through the github interface, or via:
git tag -a v0.1.1 -m "<description of version>" 2839f264af2c335dbb8c8e2beeede3aca7ee8265
git push origin v0.1.1


Rtcshell

The very first start of a shell command group for RTC access

Hi, first impressions look very good :)
The CI is failing because you need to specify that the sample you have added should be built for boards which have an RTC, this is achieved by adding these two lines to your samples .yaml
tests:
  sample.drivers.rtc:
    tags:
      - RTC
    filter: dt_alias_exists("rtc")
    depends_on: rtc

Hi, first impressions look very good :)
The CI is failing because you need to specify that the sample you have added should be built for boards which have an RTC, this is achieved by adding these two lines to your sample's .yaml
tests:
  sample.drivers.rtc:
    tags:
      - RTC
    filter: dt_alias_exists("rtc")
    depends_on: rtc

@bjarki-trackunit : Twister keeps failing due to timeout when running the sample project for native_posix. But a sample project is typically supposed to run forever, so what am I missing in respect to force twister not to timeout - or at least not regard the timeout as an error?

I see, you need to add a board which is buildable, but not testable, since we don't actually want to run the sample, just verify that it builds. To do this, add
    ...
    depends_on: rtc
    integration_platforms:
      - frdm_k64f

The frdm_k64f board is not emulatable, and will only be built, contrary to the natix_posix board which is used for unit testing, and automatically included if integration_platforms has not been specified :)

@KimBP to ease the review (and hopefully getting this merged faster), it would be better to split the PR in 2:
STM32 RTC
RTC Shell
It will help getting right reviewers on the right parts and avoid shell part blocking the stm32 driver part (and vice versa).

@erwango It was actually two PR's initially. You suggested to fix my sample issue with e.g. frdm_k64f, but since no new RTC support exists for NXP I thought by joining the two PR's I could make it work for an ST.
Still though I do get a twister error I can't explain.
I'll split the PR into two again

@erwango It was actually two PR's initially. You suggested to fix my sample issue with e.g. frdm_k64f, but since no new RTC support exists for NXP I thought by joining the two PR's I could make it work for an ST. Still though I do get a twister error I can't explain.
I'll split the PR into two again

You will need to write a hardware-independent test suite for the RTC shell. Please see my previous comment for inspiration.

Separate PR #59779 made for driver @gautierg-st, @henrikbrixandersen

driver part removed as requested.
sample project removed and replaced with test project using a rtc_fake driver

@erwango @gautierg-st @henrikbrixandersen Would you kindly re-review this PR?

Let me know when the PR is ready for review again :)

@cfriedt Could you please review this one :)

Can I do something to give this PR attention again?
What about the 2 changes requested? I think they are both fixed, but can't see how to get rid of the message

I will bring it up at tomorrows dev-review to get some attention to it, and ping @erwango @henrikbrixandersen :)


[SPARK-42730][CONNECT][DOCS] Update Spark Standalone Mode page

What changes were proposed in this pull request?
[SPARK-42730][CONNECT][DOCS]
Add start-connect-server.sh/stop-connect-server.sh to this list and cover Spark Connect sessions - other changes needed here.
Why are the changes needed?
[SPARK-42730][CONNECT][DOCS]
Add start-connect-server.sh/stop-connect-server.sh to this list and cover Spark Connect sessions - other changes needed here..

Does this PR introduce any user-facing change?
No.

How was this patch tested?
Spark Document related patch tested.

Merged to master and branch-3.5.


Utests for notifications models functions

Automated message from Dropbox CLA bot
@TomaszKolek, it looks like you've already signed the Dropbox CLA. Thanks!

I noticed that receives_offline_notifications has no tests as well. I added but reject it if you think its not needed.

Great; it's definitely worth adding tests for both while we're at it :)

Cool; I'd been imagining more of an integration test, but actually a unit test is pretty reasonable here.
You forgot to tag this as fixing #2645 :).
I've merged this after fixing that and the whitespace in the import line-wrapping.  Thanks @TomaszKolek!

@timabbott Could you describe what you actually wanted here?
I did ;-) In first commit's message

Well, I'd been imagining more of an end-to-end test for whether push notifications get sent that would result in actually running handle_push_notifications; we'll still eventually want a test that does that, but I don't think it's the right way to test this code path in particular (unit tests are good for this) :).


Fix issue where selected invited new user becomes unselected when ret…

Details
Fixed Issues
$ https://github.com/Expensify/App/issues/53899
PROPOSAL: https://github.com/Expensify/App/issues/53899#issuecomment-2543205535
Tests
Same QA step

[x] Verify that no errors appear in the JS console

Offline tests
QA Steps

Open the Expensify app.
Navigate to Workspace Settings > Members.
Go offline.
Click on Invite Member.
Invite a new user (with a new account).
Select the checkbox next to the newly invited user.
Go back online.
Verify that the invited new user remains selected.

[x] Verify that no errors appear in the JS console

PR Author Checklist

[x] I linked the correct issue in the ### Fixed Issues section above
[x] I wrote clear testing steps that cover the changes made in this PR

[x] I added steps for local testing in the Tests section
[x] I added steps for the expected offline behavior in the Offline steps section
[x] I added steps for Staging and/or Production testing in the QA steps section
[x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
[x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).

[x] I included screenshots or videos for tests on all platforms
[x] I ran the tests on all platforms & verified they passed on:

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

[x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[x] I followed proper code patterns (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method

[x] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:

[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I followed the guidelines as stated in the Review Guidelines
[x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] I verified that if a function's arguments changed that all usages have also been updated correctly
[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:

[x] I verified that all the inputs inside a form are aligned with each other.

[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native
https://github.com/user-attachments/assets/9f91a5a5-9497-4b16-864e-730482c5d5f5

Android: mWeb Chrome
https://github.com/user-attachments/assets/def84e2a-06fd-4cf5-a379-fa83e29cf6c3

iOS: Native
https://github.com/user-attachments/assets/ecd1e4b3-3f62-4b9a-a198-d46bd5406f2e

iOS: mWeb Safari
https://github.com/user-attachments/assets/a3a58b3b-e5c1-465a-811f-c54f04cd6a65

MacOS: Chrome / Safari
https://github.com/user-attachments/assets/539a75eb-87f0-4d75-948f-fe0d1023de1f

MacOS: Desktop
https://github.com/user-attachments/assets/e3c2d54c-9634-4b03-9197-aed849ef179d

Reviewer Checklist

[ ] I have verified the author checklist is complete (all boxes are checked off).
[ ] I verified the correct issue is linked in the ### Fixed Issues section above
[ ] I verified testing steps are clear and they cover the changes made in this PR

[ ] I verified the steps for local testing are in the Tests section
[ ] I verified the steps for Staging and/or Production testing are in the QA steps section
[ ] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[ ] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[ ] I checked that screenshots or videos are included for tests on all platforms
[ ] I included screenshots or videos for tests on all platforms
[ ] I verified tests pass on all platforms & I tested again on:

[ ] Android: Native
[ ] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

[ ] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
[ ] I verified proper code patterns were followed (see Reviewing the code)

[ ] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[ ] I verified that comments were added to code that is not self explanatory
[ ] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[ ] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
[ ] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[ ] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
[ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[ ] I verified the JSDocs style guidelines (in STYLE.md) were followed

[ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[ ] I verified that this PR follows the guidelines as stated in the Review Guidelines
[ ] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
[ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[ ] I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
[ ] If a new component is created I verified that:

[ ] A similar component doesn't exist in the codebase
[ ] All props are defined accurately and each prop has a /** comment above it */
[ ] The file is named correctly
[ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[ ] The only data being stored in the state is data necessary for rendering and nothing else
[ ] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[ ] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[ ] All JSX used for rendering exists in the render method
[ ] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[ ] If any new file was added I verified that:

[ ] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[ ] If a new CSS style is added I verified that:

[ ] A similar style doesn't already exist
[ ] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)

[ ] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[ ] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[ ] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[ ] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[ ] If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:

[ ] I verified that all the inputs inside a form are aligned with each other.
[ ] I added Design label and/or tagged @Expensify/design so the design team can review the changes.

[ ] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[ ] For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
[ ] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[ ] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@huult Faund issue when validation the PR - issue https://github.com/Expensify/App/issues/54489 on Desktop
https://platform.applause.com/services/links/v1/external/b3d567db94f8e34ee51fc55e86aed261b9274d599b82383e034b9b8ee69377a8

@izarutskaya Can you provide more details about the issue you’re referring to?


Reuse rest/spread element for object rest/spread

Fix #1588

Codecov Report

Merging #1778 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #1778   +/-   ##
======================================
  Coverage     100%    100%
======================================
  Files           1       1
  Lines        4050    4030   -20
  Branches      713     713
======================================
- Hits         4050    4030   -20

Impacted Files
Coverage Δ

dist/esprima.js
100% <0%> (ø)
:white_check_mark:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 860acd2...57beed1. Read the comment docs.


local contrast cuda

any plan or guidance to create a cuda version of the local enhancement function?

Not anytime soon. Anyone who wants to contribute to this, I can provide help and guidance. Please let me know


Add underlying provider name to the currency pair

Hey MoneyPHP team.
I am doing some currency conversations using the swap implementation as the baseline, which internally uses the exchange framework behind the scenes using its chain functionality.
I have been asked to save the conversion rate and the provider that was used to fetch that conversion rate somewhere. Which brings up a question that i am trying to solve "How can i get the provider that was used to fetch the conversion rate?"
I believe i have managed to solve that issue for the different implementations that we have for our exchange functionality by also allowing the "CurrencyPair" to hold that information for us.
I hope this will make it in so i can get the context i need.
If changing method signatures is considered a breaking change i would be open for changing that around.

You should solve this on your own end. Create a new (value) object and your own exchange.
<?php
use Exchanger\Contract\ExchangeRateProvider;
use Exchanger\CurrencyPair as ExchangerCurrencyPair;
use Exchanger\Exception\Exception as ExchangerException;
use Exchanger\ExchangeRateQuery;
use Money\Currency;
use Money\CurrencyPair;
use Money\Exception\UnresolvableCurrencyPairException;
use Money\Exchange;

use function assert;
use function is_numeric;
use function sprintf;

final class ProvidedCurrencyPair
{
    private CurrencyPair $pair;
    private string providerName;

    public function __construct(CurrencyPair $pair, string $providerName)
    {
        $this->pair = $pair;
        $this->providerName = $providerName;
    }

    public function getPair(): CurrencyPair
    {
         return $this->pair;
    }

    public function getProviderName(): string
    {
         return $this->providerName;
    }
}

final class MyExchange implements Exchange
{
    private ExchangeRateProvider $exchanger;

    public function __construct(ExchangeRateProvider $exchanger)
    {
        $this->exchanger = $exchanger;
    }

    public function quoteWithProviderName(Currency $baseCurrency, Currency $counterCurrency): CurrencyPair
    {
        try {
            $query = new ExchangeRateQuery(
                new ExchangerCurrencyPair($baseCurrency->getCode(), $counterCurrency->getCode())
            );
            $rate  = $this->exchanger->getExchangeRate($query);
        } catch (ExchangerException) {
            throw UnresolvableCurrencyPairException::createFromCurrencies($baseCurrency, $counterCurrency);
        }

        $rateValue = sprintf('%.14F', $rate->getValue());

        assert(is_numeric($rateValue));

        return new ProvidedCurrencyPair(
            new CurrencyPair($baseCurrency, $counterCurrency, $rateValue),
            $rate->getProviderName()
        );
    }

    public function quote(Currency $baseCurrency, Currency $counterCurrency): CurrencyPair
    {
        return $this->decoratedExchange->quoteWithProvider($baseCurrency, $counterCurrency)->getPair();
    }
}

Not dropping the final keyword, anywhere. You don't need to extend, you need a new class, e.g. ProvidedCurrencyPair as I called in the example.

@frederikbosch  Okay can you give me an example where i can get the providerName from calling.
[$converted, $pair] = (new \Money\Converter())->convertAndReturnWithCurrencyPair(new \Money\Money(1000, new \Money\Currency('EUR')), new \Money\Currency('EUR'))
Without having to reinvent the \Money\Converter class also sense i cannot see how i can just decorate it?


With layer-shell enabled and panel set on bottom layer popups should open on top layer

Right now menus are are being show under the windows.

Dups of #16


Using the gh-gomod-generate-sbom action, fails when execution the "Cheap trick" gocmd.ModWhy call

Problem
Currently I'm trying to integratie the generation of a SBOM for one of our Go repos. The gh-gomod-generate-sbom action is used. This actually fails with the error:
{"level":"error","error":"failed to download modules: command `/usr/bin/go mod why -m -vendor github.com/CycloneDX/cyclonedx-go` failed: exit status 1","time":"2022-12-16T16:28:08Z"}

Looking at this line and the subsequent call to gocmd.ModWhy the error "failed to download modules: ..." doesn't actually indicate a failure of downloading modules, but more generally a failure when running the command (in this case `go mod why -m -vendor github.com/CycloneDX/cyclonedx-go).
This will call private repo's. So in that sense, it looks like this issue #206. But that one was closed without giving a hint what goes wrong.
On a local dev machine it works.
Question
Can somebody explain the above error when running cyclonedx-gomod through a GitHub Action?
Below the contents of the action:
jobs:
  generate-sbom:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3

      - name: Install cyclonedx-gomod
        uses: CycloneDX/gh-gomod-generate-sbom@v1
        with:
          version: v1
      - name: Generate SBOM
        run: cyclonedx-gomod mod -verbose -json -output sbom.json ${{ github.workspace }}

Hello team, any updates about this issue? I tried using the docker image, tag v1.4 and I have the same error as @jeroendee reported. If I use the client locally in the laptop works well...

Is it possible to provide some kind of minimal reproducer for this? I have not been able to replicate this so far.
Generally, if a project depends on private modules, then the usual setup of GOPRIVATE etc. required for private modules is necessary to generate an SBOM for the project. If it works on your local machine, but doesn't in CI, then there's some sort of setup, config, or environment variable missing in CI, that exists on your local machine.

From my side, I couldn't try it with Github actions, but I did it using docker.
If I execute the client app locally:
go install github.com/CycloneDX/cyclonedx-gomod/cmd/cyclonedx-gomod@latest

# in the dir of my project:
cyclonedx-gomod mod -json -output bom.json .

The BOM file is generated correctly.

But then, running the docker container, using as volume the root of my project:
docker run -it \
    -v "$(pwd):/usr/src/test" \
    -v "$(pwd)/reports:/out" \
    cyclonedx/cyclonedx-gomod:v1.4 mod -json -output bom.json  /usr/src/test

I have this output:
{"level":"error","error":"failed to download modules: command `/usr/local/go/bin/go mod why -m -vendor github.com/CycloneDX/cyclonedx-go` failed: exit status 1","time":"2023-08-03T11:38:44Z"}

I don't know the root cause, but with this, I'm not sure that's related to private repos, it seems an error executing the go mod why command

I'll also see if I can improve the logging. Seems like currently we're swallowing the actual error message, which is not helpful.

Actually it is logged in debug mode (with -verbose flag):
$ docker run -it --rm -v "$(pwd):/work" cyclonedx/cyclonedx-gomod:v1.4.0 mod -verbose /work
4:00PM DBG executing command cmd="/usr/local/go/bin/go mod why -m -vendor github.com/CycloneDX/cyclonedx-go" dir=/work
4:00PM DBG failed to initialize build cache at /.cache/go-build: mkdir /.cache: permission denied
{"level":"error","error":"failed to download modules: command `/usr/local/go/bin/go mod why -m -vendor github.com/CycloneDX/cyclonedx-go` failed: exit status 1","time":"2023-08-03T16:00:03Z"}

Hi @nscuro , first, thank you for your quick response! Effectively, I added the env variable in the docker command, and it works well 🎉 , this was my probe:
docker run -it \
    -v "$(pwd):/usr/src/test" \
    -v "$(pwd)/reports:/out" \
    -e "GOCACHE=/tmp/gocache" \
   cyclonedx/cyclonedx-gomod:v1.4 mod -json -output bom.json  /usr/src/test

Thank you!!! Anyways, I saw your MR, which is merged, and I've tried also the docker latest image, works well 😄
docker run -it \
    -v "$(pwd):/usr/src/test" \
    -v "$(pwd)/reports:/out" \
    cyclonedx/cyclonedx-gomod mod -json -output /out/bom.json  /usr/src/test


Incorrect leader_vrf_0 values in chain table?

Describe the bug
Chain table format for leader_vrf_0 values has changed in recent versions and now these values appear to be incorrect as they no longer correlate with the slot battle winner.
To Reproduce
Query the chain table for slot battles where the winner had a higher leader_vrf_0 value
Eg:
sqlite3 /var/lib/cardano/cncli.db 'select winner.block_number, winner.slot_number, winner.pool_id, winner.hash, winner.leader_vrf_0, loser.leader_vrf_0 from chain winner inner join chain loser on winner.slot_number = loser.slot_number and winner.block_number = loser.block_number and winner.orphaned=0 and loser.orphaned=1 and winner.leader_vrf_0 > loser.leader_vrf_0 order by winner.block_number;'

Note that the winner should have a lower leader_vrf_0 value.  The above query should therefore retrieve cases where the slot battle subsequently became a height battle.  This happens rarely when another node builds the next block on top of the block that had the higher leader_vrf_0 value because it hadn't received the other block with the lower leader_vrf_0 value yet.  Thus what should have been a slot battle becomes a height battle and is subsequently resolved by the longest chain rule.  This should happen rarely and can be confirmed by review of log files on a node.
I noticed that the above query returns a lot more instances in more recent times.  However, upon reviewing log files all these instances are simple slot battles which should have been resolved in favour of the block with the lower leader_vrf_0 score.
Since around slot 72236516 the leader_vrf_0 value hex string is now much shorter.  In recent versions of cncli this shorter hex leader_vrf_0 value no longer appears to predict the slot battle winner.
Expected behavior
According to Duncan Coutts' statement at https://github.com/input-output-hk/ouroboros-network/issues/2913#issuecomment-816953407 slot battles should be won by the block that has the lower leader vrf score.
Additional context
It looks like the format of the chain table leader_vrf_0 values changed in recent versions of cncli.  Are these values still correct?

@AndrewWestberg would you please describe how the leader vrf value is calculated for a block or how one can independently verify it?

@TerminadaPool I'll check into this next week, but I've done a video on the babbage changes here: https://www.youtube.com/watch?v=SGNAsfsVr6Q

@AndrewWestberg Thanks.  I watch all of your excellent videos and I just watched that one again.  I wish I could understand Rust and Haskell better to be more helpful.
But maybe I can help with some investigative work.  Here is a query to retrieve the last 5 slot battles where the winner unexpectedly had a higher leader_vrf_0 value:
sqlite3 /var/lib/cardano/cncli.db 'select winner.block_number, winner.slot_number, winner.pool_id, winner.hash, winner.leader_vrf_0, loser.leader_vrf_0 from chain winner inner join chain loser on winner.slot_number = loser.slot_number and winner.block_number = loser.block_number and winner.orphaned=0 and loser.orphaned=1 and winner.leader_vrf_0 > loser.leader_vrf_0 order by winner.block_number desc limit 5;'

Here is what my logs show for the latest slot battle:
Sep 30 07:33:03 relay1 cardano-node[995341]: [relay1:cardano.node.ChainDB:Notice:169] [2022-09-29 21:33:03.70 UTC] Chain extended, new tip: 7e509ed514b0a5ca12cbc2efa7b9b2a3c995f3fecd9cd77a3577b3b6edcc89d9 at slot 72920892
Sep 30 07:33:03 relay1 cardano-node[995341]: [relay1:cardano.node.ChainDB:Info:169] [2022-09-29 21:33:03.79 UTC] Block fits onto some fork: 9ae2d5b41718c4f132e59d2450a12455c89ab7b3ece15cd28bd6ac364cd7b742 at slot 72920892

Sep 30 07:33:03 relay1 cardano-node[995341]: [relay1:cardano.node.ChainDB:Notice:169] [2022-09-29 21:33:03.82 UTC] Switched to a fork, new tip: 9ae2d5b41718c4f132e59d2450a12455c89ab7b3ece15cd28bd6ac364cd7b742 at slot 72920892

And the second latest slot battle:
Sep 30 07:26:52 relay1 cardano-node[995341]: [relay1:cardano.node.ChainDB:Notice:169] [2022-09-29 21:26:52.22 UTC] Chain extended, new tip: 42a021dddc771a4b210840e0bf8e4270f1c4fda769a0813e0ea3f72a44fe805c at slot 72920520
Sep 30 07:26:52 relay1 cardano-node[995341]: [relay1:cardano.node.ChainDB:Info:169] [2022-09-29 21:26:52.44 UTC] Block fits onto some fork: 5e636930f202839fcddf0447e4df28a65d590439fb8ea76c2a27652a8e6b3861 at slot 72920520

Sep 30 07:26:52 relay1 cardano-node[995341]: [relay1:cardano.node.ChainDB:Notice:169] [2022-09-29 21:26:52.53 UTC] Switched to a fork, new tip: 5e636930f202839fcddf0447e4df28a65d590439fb8ea76c2a27652a8e6b3861 at slot 72920520

In both cases the second block was received only a few milliseconds (9,22) later and it was immediately preferred despite this second block having a higher leader_vrf_0 value in the cncli database.
Possible explanations:

The leader_vrf_0 values stored in cncli database are not the same as the leader vrf values the node is using for comparison.
The node is not doing a leader vrf comparison and instead preferring the second block received for some other reason.  Which would be weird and possibly bad!

Sorry to give you an extra concern to look at when you are busy.

I looked at logs on my relay3, which is on the other side of the world.  It happened to receive the latest slot battle blocks in the opposite order to my relay1.  And, it didn't extend its tip with the second block.  Notice the difference between the logs below and the first output in my previous comment which logs the same blocks.
Sep 30 07:33:03 relay3 cardano-node[1012155]: [relay3:cardano.node.ChainDB:Notice:306] [2022-09-29 21:33:03.33 UTC] Chain extended, new tip: 9ae2d5b41718c4f132e59d2450a12455c89ab7b3ece15cd28bd6ac364cd7b742 at slot 72920892

Sep 30 07:33:03 relay3 cardano-node[1012155]: [relay3:cardano.node.ChainDB:Info:306] [2022-09-29 21:33:03.92 UTC] Block fits onto some fork: 7e509ed514b0a5ca12cbc2efa7b9b2a3c995f3fecd9cd77a3577b3b6edcc89d9 at slot 72920892

But the tip wasn't extended with that 7e509 block.

Then the chain moves on with next block as usual

Sep 30 07:33:05 relay3 cardano-node[1012155]: [relay3:cardano.node.BlockFetchDecision:Info:316] [2022-09-29 21:33:05.15 UTC] [TraceLabelPeer (ConnectionId {localAddress = <IP_ADDRESS>:42419, remoteAddress = <IP_ADDRESS>:3001}) (Right [At (Block {blockPointSlot = SlotNo 72920894, blockPointHash = a9bf4bca57148425216eed70484083c8c2e8996a5ab>
Sep 30 07:33:05 relay3 cardano-node[1012155]: [relay3:cardano.node.ChainDB:Notice:306] [2022-09-29 21:33:05.30 UTC] Chain extended, new tip: a9bf4bca57148425216eed70484083c8c2e8996a5ab6a9f9d33e4fce7ebf0bad at slot 72920894

Block 7e509... was received 59ms after 9ae2d... but relay3 did not adopt block 7e509...  So the order of block receipt doesn't matter.  In both relays block 9ae2d was preferred.  I must presume that both these nodes preferred block 9ae2d because its leader vrf score was actually lower.  Whereas the cncli database shows block 7e509 as having the lower leader_vrf_0 score:
sqlite3 /var/lib/cardano/cncli.db 'select block_number, slot_number, hash, leader_vrf_0, orphaned from chain where slot_number=72920892;'

7821110|72920892|7e509ed514b0a5ca12cbc2efa7b9b2a3c995f3fecd9cd77a3577b3b6edcc89d9|000037a19fc8c185e59e66572121bd8697c254726ccb89c44e151388dc36112e|1
7821110|72920892|9ae2d5b41718c4f132e59d2450a12455c89ab7b3ece15cd28bd6ac364cd7b742|0005141d6304f92bd4a189b79d93dced2b5dfe5daff3af2b8e38cd9f11c59c94|0

Therefore the likely explanation is:

leader_vrf_0 values stored in cncli database are not the same as the correct leader vrf values nodes are using for comparison.

@TerminadaPool There is nothing wrong in cncli. I believe you've actually found a bug in cardano-node. It appears that the node is incorrectly using the block_vrf_0 value for determining who wins the slot battle instead of the leader_vrf_0 value.
This is incorrect because it removes the slight advantage that small pools had in slot leadership and instead makes it entirely random.
I queried my cncli db in a similar manner as yours and determined a fairly random pattern of winners when I looked at leader_vrf_0. Out of the 30 most recent slot battles queried, 13 were showing the higher leader_vrf_0 value winning.
When you compare the block_vrf_0 values for all 30, they are perfectly in line with the lower block_vrf_0 winning each time.

@TerminadaPool I'll reach out to IOG to see what they want to do about this.

@TerminadaPool Please create a github issue here and link this ticket in it. https://github.com/input-output-hk/ouroboros-network/issues

Bug confirmed by Jared from IOG.  In Alonzo and earlier. the leader_vrf value was being used:
https://github.com/input-output-hk/ouroboros-network/blob/9249a70ed9e2365f3963e47cb31b4b1589bca8f6/ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Protocol/TPraos.hs#L111
In Babbage, the raw vrf value is being passed instead of calculating the leader_vrf from it.
https://github.com/input-output-hk/ouroboros-network/blob/9249a70ed9e2365f3963e47cb31b4b1589bca8f6/ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Protocol/Praos.hs#L141

watch it in action:  https://www.youtube.com/shorts/pXGQQmCneM4

Now that I've looked into this, I know enough to be able to make an issue myself.

Apologies to @AndrewWestberg for thinking the bug was in cncli.

@TerminadaPool no need to apologize whatsoever. You found a bug. This will forever be a contribution to improving cardano that you have made.

In case this "bug" was instead a design decision:  I pointed out that leader VRF value is not possible to game whereas block VRF can be manipulated by custom software doing transaction selecting.
https://github.com/input-output-hk/ouroboros-network/issues/4051#issuecomment-1264211740

I have been corrected by @JaredCorduan

the value of the VRF in the block header depends on:

the epoch nonce
the slot
the private VRF key

I hope that "corrected" reads like "helped" to most folks! None of this is obvious, there is a lot of technical detail here which is not easy to discover, and my hope is that I can empower y'all.

Closing this issue on the cncli side for now. Thanks for reporting @TerminadaPool and thanks @JaredCorduan for taking the ball.


Move model weights used in nightly regression to weka

Ran some benchmarks and had some discussions  with @teijo regarding optimizing weka performance. Turns out weka is really fast for files that we use frequently, but very slow for infrequently used files:
"Hot" data:
billteng@e03cs01:/mnt/MLPerf/tt_dnn-models/Mistral/mistral-7B-v0.1$ dd if=./consolidated.00.pth of=/dev/null bs=1M count=1024
1024+0 records in
1024+0 records out
1073741824 bytes (1.1 GB, 1.0 GiB) copied, 0.312791 s, 3.4 GB/s

"Cold" data:
billteng@e03cs01:/mnt/MLPerf/tt_dnn-models/llama-2/llama-2-7b$ dd if=./pytorch_model-00002-of-00002.bin of=/dev/null bs=1M count=1024
1024+0 records in
1024+0 records out
1073741824 bytes (1.1 GB, 1.0 GiB) copied, 342.247 s, 3.1 MB/s

@teijo has increased data retention to 1 month so any file we touch in /mnt/MLPerf should be very fast now.
We should ensure ALL models used in nightly have files on weka.
For example from run_models.sh, I see:
def run_whisper_decoder_layer(layer, device):
    model = WhisperModel.from_pretrained("openai/whisper-tiny.en")

def test_vit_image_classification(device, hf_cat_image_sample_input, pcc=0.95):
    image = hf_cat_image_sample_input

    with torch.no_grad():
        HF_model = HF_ViTForImageClassication.from_pretrained(
            "google/vit-base-patch16-224"
        )
        image_processor = HF_AutoImageProcessor.from_pretrained(
            "google/vit-base-patch16-224"
        )

def test_roberta_classification_head(device):
    torch.manual_seed(1234)
    base_address = f"classifier"
    with torch.no_grad():
        model = RobertaForSequenceClassification.from_pretrained(
            "cardiffnlp/twitter-roberta-base-emotion"
        )

We have a fixture for this

fyi @tt-rkim

We are thinking of moving weka check from cleanup and teardown script into fixtures. This is to reduce runner startup failures + enforce that all model files use weka exclusively as cache.

@TT-billteng
As part of this effort, Bill will try to separate the model tests from the rest of the post commit tests. Furthermore, we will push to enforce that /mnt/MLPerf is only used in model pipelines/tests. These tests will also never see the light of day in regular post commit unless in a special model designated pipeline.

from what I can tell, only the frequent pipeline touches /mnt/MLPerf? @boris-drazic @farbabi

from what I can tell, only the frequent pipeline touches /mnt/MLPerf? @boris-drazic @farbabi

@TT-billteng frequents and perf pipelines touch weka.


Connector should not log "start transfer" when it's actually not starting any transfer

Bug Report
Describe the Bug
The EDC logs pretty early that is starting a transfer. In combination with the missing exception logging, the whole log of a failed transfer start might look like this
[2022-05-12 08:57:27] [FINE   ] Starting transfer for asset

Expected Behavior

start transfer should only be logged if a transfer is actually started

Observed Behavior

connector logs that it started the transfer without actually starting anything.

Steps to Reproduce
Steps to reproduce the behavior:

Make it somehow that the transfer process cannot be persisted (easy with external SQL storage)
service.initiateTransfer will throw an unlogged exception and no transfer started

Context Information

    @POST
    @Override
    public TransferId initiateTransfer(@Valid TransferRequestDto transferRequest) {
        var dataRequest = Optional.ofNullable(transformerRegistry.transform(transferRequest, DataRequest.class))
                .filter(AbstractResult::succeeded).map(AbstractResult::getContent);
        if (dataRequest.isEmpty()) {
            throw new IllegalArgumentException("Error during transforming TransferRequestDto into DataRequest");
        }
        monitor.debug("Starting transfer for asset " + transferRequest.getAssetId());

        var result = service.initiateTransfer(dataRequest.get());
        if (result.succeeded()) {
            monitor.debug(format("Transfer process initialised %s", result.getContent()));
            return new TransferId(result.getContent());
        } else {
            String message = format("Error during initiating the transfer with assetId: %s",
                    transferRequest.getAssetId());
            monitor.severe(message);
            throw new EdcException(message);
        }
    }

Detailed Description

Possible Implementation

Dominik Pinsel<EMAIL_ADDRESS>Mercedes-Benz Tech Innovation GmbH, legal info/Impressum

We should break this issue down:

The debug message is correctly emitted at the current point in the transfer process. It could be clarified to "Initiating..." instead of "Starting..." but those terms are largely synonymous.

The EdcApiExceptionMapper should log the exception. However, the client will receive an error, which is correct.

I've changed this issue to an enhancement request.


New command: teams conversationmember add

Usage
teams conversationmember add [options]
Alias
teams conversationmenber set [options]
Description
Adds or sets a conversationmember in a private channel
Options

Option
Description

--teamId [teamId]
The Team's ID. Specify either the teamName or teamId

--teamName [teamName]
The Team's displayname. The Team's ID. Specify either the teamName or teamId

--channelId [channelId]
The private channel's ID. Specify either the channelName or channelId

--channelName [channelName]
The private channel's name. Specify either the channelName or channelId

--userId [userId]
The user's ID or userprincipalname. You can also pass a comma separated list of userIds

--userDisplayName [userDisplayName]
The displayName of a user. You can also pass a comma separated list of displaynames.

--owner
Assign the user the owner role. Defaults to member permissions

Note

At least one owner must be assigned to a private channel.
You can only add members and owners of a Team to a private channel.

Additional Information
https://docs.microsoft.com/en-us/graph/api/team-post-members?view=graph-rest-1.0&tabs=http

Looking at the spec, let's allow referring to team, channel and user by their name as well to make it easier to use.

@waldekmastykarz as in #1880 and #1879
I am not sure about the DisplayName of the Team. Is not unique and therefore not super appropriate I believe. Channel name can be considered because it is unique.
Not sure about the DisplayName of the user. I think the userprincipalname is more safe. Do we have other examples regarding this?
What came to my mind is that you can pass multiple userIds as an array. I can also perform the operation oneshot.
In a real world scenario this would mean that I have to run two commands if I want to set the owners and members of a team
teams conversationmembers add --teamId [teamId] --channelId [channelId] --userId [111, 112, 113, 114] --owner
teams conversationmembers add --teamId [teamId] --channelId [channelId] --userId [555, 556, 557]
This would add 3 members and two 4 owners to the private channel.

Updated command name to singular to match our naming convention.

I am not sure about the DisplayName of the Team. Is not unique and therefore not super appropriate I believe. Channel name can be considered because it is unique.

See remark on #1880

Not sure about the DisplayName of the user. I think the userprincipalname is more safe. Do we have other examples regarding this?

Good point. I don't think we have a sample with UPN, but it makes sense to use it over the display name

What came to my mind is that you can pass multiple userIds as an array. I can also perform the operation oneshot.

I don't think you can pass an array as an argument in bash. The most we can do is a comma-separated string which we then break up into an array. It absolutely makes sense to allow the ability to pass multiple users

Hi @sebastienlevert,
is this a command that you would like to pick up? It is using the endpoints you already used in the conversationlist commands.
An important difference here is that you can only execute the operation on private channels only.
Thank you for your feedback.
br,
Patrick

I can, but it will have to wait to next week!
Please assign me!

@sebastienlevert Thank you. Highly appreciated

Just wanted to check in on this one. Still on my table, but looking for more cycles. If someone want it, feel free to raise you hands. If not, I'll keep it and come to it somewhere before the end of this year! Sorry about it!

No need to be sorry. Life happens, especially around switching jobs 😁 Shall we set it as open for grabs and if it's still available when you have time, you can take it back. Deal?

Deal!

Hey Guys! I would like to work on this if it's still available.

Hey Guys! I would like to work on this if it's still available.

Hi @aakashbhardwaj619,
thank you for reaching us out. It's all yours.
Feel free to contact us if you have some questions.
br,
Patrick

Hi @aakashbhardwaj619,
thank you for reaching us out. It's all yours.
Feel free to contact us if you have some questions.
br,
Patrick

Hi @plamber! Could you please confirm if user's UPN should be used instead of the displayName? And if so, should I update the userDisplayName option with userName like in the user app list command?

Hi @aakashbhardwaj619,
the endpoint changed lately. What is your recommendation based on what you saw so far?
br,
Patrick

To add the user to the Private Channel, I'm using the user id as specified in this example and it is working fine.
To get the userId from UPN, we would need to execute the query<EMAIL_ADDRESS>and it would return the appropriate user object.
To get the userId from displayName, we would have to execute the query https://graph.microsoft.com/v1.0/users?$filter=displayName eq 'admin' and it would return an array of the users with that displayName. In this case, we would have to check the item count and ask the user to disambiguate if multiple users are found.
I think it would be better to use the UPN and update the option name to userName like in the other command.

I can recall that we had this discussion already somewhere. Indeed, for simplicity we should add these queries and add to the docs the information that this operation is less performant.
In case there are two users with the same name, i would return a warning and do not perform the operation

So the suggestion is to use the userDisplayName option with displayName filter query and give the warning if multiple users are found, right?
I think if we use the userPrincipalName instead of displayName, then we would not need to add the "less performant" part in the docs as it would return only a single user with the unique UPN. Let me know what you think @plamber, @waldekmastykarz. 😊

The idea was to use the userId for that. The specs say:
The user's ID or userprincipalname. You can also pass a comma separated list of userIds
Is the endpoint accepting the userprincipalname without additional queries in the backend or only the userId?

Ideally, we should accept all values. Display name might be tricky, especially for management purposes so settling on user ID and upn are a good balance of convenience and reliability. If you want to look up person's upn based on their preferred/display name, that's a separate operation.

Hey @aakashbhardwaj619, are you still working on this?

Hi @waldekmastykarz I was working on the test cases for this command, but then had a couple of busy weeks. I'll try to complete this over the weekend.

Awesome! Appreciate you sticking with us 👍